From 7b83cfab38aa32f1c7d04cb12b7a2549d475eb94 Mon Sep 17 00:00:00 2001
From: Jonas Kattendick <kattendick@bde-software.com>
Date: Sun, 17 Aug 2025 13:55:24 +0200
Subject: [PATCH] fix: handle stdio properly

---
 README.md                |   2 +-
 example.php              |  26 --------
 examples/hello-world.php |  24 +++++++
 examples/plumbing.php    |  17 +++++
 examples/stdio.php       |  35 ++++++++++
 src/Child.php            |  32 ++++++---
 src/ChildException.php   |   8 +++
 src/Command.php          | 141 +++++++++++++++++++++++++++------------
 src/CommandException.php |   2 +-
 src/ExitStatus.php       |  18 +++++
 src/Output.php           |   2 +-
 src/Stdio.php            |  48 ++++++++++---
 src/StdioFile.php        |  25 -------
 src/StreamReadable.php   |  22 +++++-
 src/StreamWritable.php   |  19 +++++-
 15 files changed, 302 insertions(+), 119 deletions(-)
 delete mode 100644 example.php
 create mode 100644 examples/hello-world.php
 create mode 100644 examples/plumbing.php
 create mode 100644 examples/stdio.php
 create mode 100644 src/ChildException.php
 create mode 100644 src/ExitStatus.php
 delete mode 100644 src/StdioFile.php

diff --git a/README.md b/README.md
index b829f00..bbff83c 100644
--- a/README.md
+++ b/README.md
@@ -11,7 +11,7 @@ use Nih\CommandBuilder\Stdio;
 require_once __DIR__ . '/vendor/autoload.php';
 
 $child = (new Command('/usr/bin/cat'))
-    ->stdin(Stdio::piped('r'))
+    ->stdin(Stdio::piped())
     ->spawn(shell: false);
 
 $child->stdin?->write('Hello, this is pretty cool.');
diff --git a/example.php b/example.php
deleted file mode 100644
index 5c17e3d..0000000
--- a/example.php
+++ /dev/null
@@ -1,26 +0,0 @@
-<?php
-
-use Nih\CommandBuilder\Command;
-use Nih\CommandBuilder\Stdio;
-
-require_once __DIR__ . '/vendor/autoload.php';
-
-$output = (new Command('echo'))
-    ->arg('-n')
-    ->arg('-')
-    ->arg("./ \$wow     '''/stdout.txt")
-    ->stderr(Stdio::null())
-    ->output();
-
-var_dump($output);
-
-$child = (new Command('/usr/bin/cat'))
-    ->stdin(Stdio::piped('r'))
-    ->spawn(shell: false);
-
-$child->stdin?->write('Hello, this is pretty cool.');
-$child->stdin?->close();
-
-$output = $child->output();
-
-var_dump($output);
diff --git a/examples/hello-world.php b/examples/hello-world.php
new file mode 100644
index 0000000..59a4cf1
--- /dev/null
+++ b/examples/hello-world.php
@@ -0,0 +1,24 @@
+<?php
+
+use Nih\CommandBuilder\Command;
+
+require_once __DIR__ . '/../vendor/autoload.php';
+
+$output = (new Command('echo'))
+    ->arg('Hello, World!')
+    ->output();
+
+var_dump($output);
+
+// object(Nih\CommandBuilder\Output)#9 (3) {
+//   ["stdout"]=>
+//   string(14) "Hello, World!
+// "
+//   ["stderr"]=>
+//   string(0) ""
+//   ["code"]=>
+//   object(Nih\CommandBuilder\ExitStatus)#8 (1) {
+//     ["code"]=>
+//     int(0)
+//   }
+// }
diff --git a/examples/plumbing.php b/examples/plumbing.php
new file mode 100644
index 0000000..d7d0eea
--- /dev/null
+++ b/examples/plumbing.php
@@ -0,0 +1,17 @@
+<?php
+
+use Nih\CommandBuilder\Command;
+use Nih\CommandBuilder\Stdio;
+
+require_once __DIR__ . '/../vendor/autoload.php';
+
+$echo = (new Command('echo'))
+    ->arg('Hello, World!')
+    ->stdout(Stdio::piped())
+    ->spawn();
+
+$cat = (new Command('cat'))
+    ->stdin(Stdio::stream($echo->stdout))
+    ->status();
+
+// Prints "Hello, World!\n"
diff --git a/examples/stdio.php b/examples/stdio.php
new file mode 100644
index 0000000..efc7c1c
--- /dev/null
+++ b/examples/stdio.php
@@ -0,0 +1,35 @@
+<?php
+
+use Nih\CommandBuilder\Command;
+use Nih\CommandBuilder\Stdio;
+
+require_once __DIR__ . '/../vendor/autoload.php';
+
+$child = (new Command('cat'))
+    ->stdin(Stdio::piped())
+    ->stdout(Stdio::piped())
+    ->spawn();
+
+$child->stdin?->write('Hello, World!');
+$output = $child->waitWithOutput();
+
+var_dump($output);
+
+// object(Nih\CommandBuilder\Output)#6 (3) {
+//   ["stdout"]=>
+//   string(13) "Hello, World!"
+//   ["stderr"]=>
+//   NULL
+//   ["code"]=>
+//   object(Nih\CommandBuilder\ExitStatus)#4 (1) {
+//     ["code"]=>
+//     int(0)
+//   }
+// }
+
+(new Command('echo'))
+    ->arg('Hello, World!')
+    ->stdout(Stdio::inherit())
+    ->status();
+
+// Hello, World!
diff --git a/src/Child.php b/src/Child.php
index cd0eb4c..50d9480 100644
--- a/src/Child.php
+++ b/src/Child.php
@@ -19,31 +19,47 @@ final class Child
 
     public function id(): int
     {
+        if (!is_resource($this->proc)) {
+            throw new ChildException('Resource was already closed');
+        }
+
         $status = proc_get_status($this->proc);
         return $status['pid'];
     }
 
-    public function output(): Output
+    public function waitWithOutput(): Output
     {
+        if (!is_resource($this->proc)) {
+            throw new ChildException('Resource was already closed');
+        }
+
+        // Avoid possible deadlock before waiting.
+        $this->stdin?->close();
+
         $stdout = $this->stdout?->getContents();
         $stderr = $this->stderr?->getContents();
-        $code = proc_close($this->proc);
+        $code = new ExitStatus(proc_close($this->proc));
 
         return new Output($stdout, $stderr, $code);
     }
 
-    public function status(): int
+    public function wait(): ExitStatus
     {
-        return proc_close($this->proc);
-    }
+        // Avoid possible deadlock before waiting.
+        $this->stdin?->close();
 
-    public function wait(): void
-    {
-        proc_close($this->proc);
+        return new ExitStatus(proc_close($this->proc));
     }
 
     public function kill(): bool
     {
         return proc_terminate($this->proc, 10);
     }
+
+    public function __destruct()
+    {
+        if (is_resource($this->proc)) {
+            proc_close($this->proc);
+        }
+    }
 }
diff --git a/src/ChildException.php b/src/ChildException.php
new file mode 100644
index 0000000..5bc1bc9
--- /dev/null
+++ b/src/ChildException.php
@@ -0,0 +1,8 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Nih\CommandBuilder;
+
+class ChildException extends CommandException
+{}
diff --git a/src/Command.php b/src/Command.php
index 8773613..56da6da 100644
--- a/src/Command.php
+++ b/src/Command.php
@@ -6,9 +6,10 @@ namespace Nih\CommandBuilder;
 
 use RuntimeException;
 use Stringable;
+use TypeError;
 use ValueError;
 
-final class Command
+final class Command implements Stringable
 {
     public readonly string $command;
 
@@ -48,7 +49,7 @@ final class Command
         return $this;
     }
 
-    public function cwd(string|Stringable $cwd): static
+    public function currentDir(string|Stringable $cwd): static
     {
         $this->cwd = (string) $cwd;
         return $this;
@@ -72,43 +73,118 @@ final class Command
         return $this;
     }
 
-    public function stdin(Stdio $in): static
+    public function stdin(Stdio $stdin): static
     {
-        $this->stdin = $in;
+        $stdin = match ($stdin->type) {
+            Stdio::INHERIT => Stdio::stream(STDIN),
+            Stdio::PIPE => new Stdio(Stdio::PIPE, ['pipe', 'r']),
+            default => $stdin,
+        };
+
+        $this->stdin = $stdin;
         return $this;
     }
 
-    public function stdout(Stdio $out): static
+    public function stdout(Stdio $stdout): static
     {
-        $this->stdout = $out;
+        $stdout = match ($stdout->type) {
+            Stdio::INHERIT => Stdio::stream(STDOUT),
+            Stdio::PIPE => new Stdio(Stdio::PIPE, ['pipe', 'w']),
+            default => $stdout,
+        };
+
+        $this->stdout = $stdout;
         return $this;
     }
 
-    public function stderr(Stdio $err): static
+    public function stderr(Stdio $stderr): static
     {
-        $this->stderr = $err;
+        $stderr = match ($stderr->type) {
+            Stdio::INHERIT => Stdio::stream(STDERR),
+            Stdio::PIPE => new Stdio(Stdio::PIPE, ['pipe', 'w']),
+            default => $stderr,
+        };
+
+        $this->stderr = $stderr;
         return $this;
     }
 
+    // TODO: Allow capturing arbitrary descriptors (proc_open supports this)?
+    // public function descriptor(int $fd, Stdio $stdio): static
+    // {
+    //     match ($fd) {
+    //         0 => $this->stdin($stdio),
+    //         1 => $this->stdout($stdio),
+    //         2 => $this->stderr($stdio),
+    //         default => $this->fd[$fd] = $stdio,
+    //     };
+    //
+    //     return $this;
+    // }
+
     /**
      * @param bool $shell Run the command with or without a shell
      */
     public function spawn(bool $shell = true): Child
     {
-        $fd[0] = $this->stdin instanceof Stdio
-            ? $this->stdin->getDescriptorSpec()
-            : ['pipe', 'r'];
+        return $this->spawnWithDescriptorSpec($shell, [
+            $this->stdin instanceof Stdio
+                ? $this->stdin->descriptorSpec
+                : STDIN,
+            $this->stdout instanceof Stdio
+                ? $this->stdout->descriptorSpec
+                : STDOUT,
+            $this->stderr instanceof Stdio
+                ? $this->stderr->descriptorSpec
+                : STDERR,
+        ]);
+    }
 
-        $fd[1] = $this->stdout instanceof Stdio
-            ? $this->stdout->getDescriptorSpec()
-            : ['pipe', 'w'];
+    /**
+     * @param bool $shell Run the command with or without a shell
+     */
+    public function status(bool $shell = true): ExitStatus
+    {
+        return $this->spawn($shell)->wait();
+    }
 
-        $fd[2] = $this->stderr instanceof Stdio
-            ? $this->stderr->getDescriptorSpec()
-            : ['pipe', 'w'];
+    /**
+     * @param bool $shell Run the command with or without a shell
+     */
+    public function output(bool $shell = true): Output
+    {
+        return $this->spawnWithDescriptorSpec($shell, [
+            $this->stdin instanceof Stdio
+                ? $this->stdin->descriptorSpec
+                : ['pipe', 'r'],
+            $this->stdout instanceof Stdio
+                ? $this->stdout->descriptorSpec
+                : ['pipe', 'w'],
+            $this->stderr instanceof Stdio
+                ? $this->stderr->descriptorSpec
+                : ['pipe', 'w'],
+        ])->waitWithOutput();
+    }
+
+    public function __toString(): string
+    {
+        return implode(' ', array_map(escapeshellarg(...), $this->args));
+    }
+
+    private function spawnWithDescriptorSpec(bool $shell, array $descriptorSpec): Child
+    {
+        foreach ($descriptorSpec as $descriptor => $spec) {
+            if (!is_array($spec) && !is_resource($spec)) {
+                throw new CommandException(sprintf(
+                    'Descriptor %d is not a valid stream resource: %s',
+                    $descriptor,
+                    get_debug_type($spec),
+                ));
+            }
+        }
 
         if ($shell) {
-            $command = implode(' ', array_map(escapeshellarg(...), $this->args));
+            $command = (string) $this;
         } else if (is_executable($this->command)) {
             $command = $this->args;
         } else {
@@ -118,9 +194,10 @@ final class Command
             ));
         }
 
-        $proc = proc_open($command, $fd, $pipes, $this->cwd, $this->envs);
+        $proc = proc_open($command, $descriptorSpec, $pipes, $this->cwd, $this->envs);
+
         if ($proc === false) {
-            throw new RuntimeException();
+            throw new RuntimeException('Failed proc_open');
         }
 
         $stdin = array_key_exists(0, $pipes)
@@ -137,28 +214,4 @@ final class Command
 
         return new Child($stdin, $stdout, $stderr, $proc);
     }
-
-    /**
-     * @param bool $shell Run the command with or without a shell
-     */
-    public function output(bool $shell = true): Output
-    {
-        return $this->spawn($shell)->output();
-    }
-
-    /**
-     * @param bool $shell Run the command with or without a shell
-     */
-    public function status(bool $shell = true): int
-    {
-        return $this->spawn($shell)->status();
-    }
-
-    /**
-     * @param bool $shell Run the command with or without a shell
-     */
-    public function wait(bool $shell = true): void
-    {
-        $this->spawn($shell)->wait();
-    }
 }
diff --git a/src/CommandException.php b/src/CommandException.php
index 0faec8c..8c109b0 100644
--- a/src/CommandException.php
+++ b/src/CommandException.php
@@ -6,5 +6,5 @@ namespace Nih\CommandBuilder;
 
 use RuntimeException;
 
-final class CommandException extends RuntimeException
+class CommandException extends RuntimeException
 {}
diff --git a/src/ExitStatus.php b/src/ExitStatus.php
new file mode 100644
index 0000000..bf891d9
--- /dev/null
+++ b/src/ExitStatus.php
@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Nih\CommandBuilder;
+
+final class ExitStatus
+{
+    public function __construct(
+        public readonly int $code,
+    ) {
+    }
+
+    public function success(): bool
+    {
+        return $this->code === 0;
+    }
+}
diff --git a/src/Output.php b/src/Output.php
index c016c68..dfb17da 100644
--- a/src/Output.php
+++ b/src/Output.php
@@ -9,7 +9,7 @@ final class Output
     public function __construct(
         public readonly ?string $stdout,
         public readonly ?string $stderr,
-        public readonly int $code,
+        public readonly ExitStatus $code,
     ) {
     }
 }
diff --git a/src/Stdio.php b/src/Stdio.php
index 04f9a9e..dff3063 100644
--- a/src/Stdio.php
+++ b/src/Stdio.php
@@ -6,27 +6,55 @@ namespace Nih\CommandBuilder;
 
 use Stringable;
 
-abstract class Stdio
+final class Stdio
 {
-    abstract public function getDescriptorSpec(): array;
+    public const INHERIT = 0;
+    public const PIPE = 1;
+    public const FILE = 2;
+    public const STREAM = 3;
 
-    public static function file(string|Stringable $file, string $mode): self
-    {
-        return new StdioFile((string) $file, $mode);
+    /**
+     * @param null
+     *   |resource
+     *   |array{0: 'file', 1: string, 2: string}
+     *   |array{0: 'pipe',1: string} $descriptorSpec
+     */
+    public function __construct(
+        public readonly int $type,
+        public readonly mixed $descriptorSpec,
+    ) {
     }
 
-    public static function piped(string $mode): self
+    public static function inherit(): self
     {
-        return new StdioPiped($mode);
+        return new self(self::INHERIT, null);
     }
 
-    public static function inherit(): null
+    public static function piped(): self
     {
-        return null;
+        return new self(self::PIPE, ['pipe', 'r']);
     }
 
     public static function null(): self
     {
-        return new StdioFile('/dev/null', 'a+');
+        return self::file('/dev/null', 'a+');
     }
+
+    public static function file(string|Stringable $file, string $mode): self
+    {
+        return new self(self::FILE, ['file', (string) $file, $mode]);
+    }
+
+    /**
+     * @param resource|StreamReadable|StreamWritable $stream
+     */
+    public static function stream($stream): self
+    {
+        if (is_object($stream)) {
+            $stream = $stream->stream;
+        }
+
+        return new self(self::STREAM, $stream);
+    }
+
 }
diff --git a/src/StdioFile.php b/src/StdioFile.php
deleted file mode 100644
index 9a0b2e4..0000000
--- a/src/StdioFile.php
+++ /dev/null
@@ -1,25 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Nih\CommandBuilder;
-
-use ValueError;
-
-final class StdioFile extends Stdio
-{
-    public function __construct(
-        public readonly string $file,
-        public readonly string $mode,
-    ) {
-        match ($mode) {
-            'r', 'w', 'a', 'x', 'r+', 'w+', 'a+', 'x+' => null,
-            default => throw new ValueError('Invalid mode: ' . $mode),
-        };
-    }
-
-    public function getDescriptorSpec(): array
-    {
-        return ['file', $this->file, $this->mode];
-    }
-}
diff --git a/src/StreamReadable.php b/src/StreamReadable.php
index a63ec23..4e02b48 100644
--- a/src/StreamReadable.php
+++ b/src/StreamReadable.php
@@ -9,20 +9,40 @@ trait StreamReadable
     /**
      * @param resource $stream
      */
-    public function __construct(private $stream)
+    public function __construct(public readonly mixed $stream)
     {
     }
 
     public function read(int $length): ?string
     {
+        if (!is_resource($this->stream)) {
+            throw new CommandException('Cannot read from closed stream');
+        }
+
         return fread($this->stream, $length) ?: null;
     }
 
     public function getContents(?int $length = null, int $offset = -1): ?string
     {
+        if (!is_resource($this->stream)) {
+            throw new CommandException('Cannot read from closed stream');
+        }
+
         $contents = stream_get_contents($this->stream, $length, $offset);
         return $contents === false
             ? null
             : $contents;
     }
+
+    public function close(): bool
+    {
+        return is_resource($this->stream)
+            ? fclose($this->stream)
+            : true;
+    }
+
+    public function __destruct()
+    {
+        $this->close();
+    }
 }
diff --git a/src/StreamWritable.php b/src/StreamWritable.php
index 3189b28..34e9a83 100644
--- a/src/StreamWritable.php
+++ b/src/StreamWritable.php
@@ -9,23 +9,38 @@ trait StreamWritable
     /**
      * @param resource $stream
      */
-    public function __construct(private $stream)
+    public function __construct(public readonly mixed $stream)
     {
     }
 
     public function write(string $data, ?int $length = null): ?int
     {
+        if (!is_resource($this->stream)) {
+            throw new CommandException('Cannot write to closed stream');
+        }
+
         $bytes = fwrite($this->stream, $data, $length) ?: null;
         return $bytes === false ? null : $bytes;
     }
 
     public function flush(): bool
     {
+        if (!is_resource($this->stream)) {
+            throw new CommandException('Cannot flush closed stream');
+        }
+
         return fflush($this->stream);
     }
 
     public function close(): bool
     {
-        return fclose($this->stream);
+        return is_resource($this->stream)
+            ? fclose($this->stream)
+            : true;
+    }
+
+    public function __destruct()
+    {
+        $this->close();
     }
 }