Internal refactoring to reuse file descriptor close logic

Author: clue

composer.json

@@ -11,7 +11,8 @@
         }
     ],
     "autoload": {
-        "psr-4": { "Clue\\React\\SshProxy\\": "src/" }
+        "psr-4": { "Clue\\React\\SshProxy\\": "src/" },
+        "files": [ "src/Io/functions.php" ]
     },
     "require": {
         "php": ">=5.3",

src/Io/functions.php

@@ -0,0 +1,47 @@
+<?php
+
+namespace Clue\React\SshProxy\Io;
+
+use React\ChildProcess\Process;
+
+/**
+ * Returns a list of active file descriptors (may contain bogus entries)
+ *
+ * @param string $path
+ * @return array
+ * @internal
+ */
+function fds($path = '/proc/self/fd')
+{
+    // try to get list of all open FDs (Linux only) or simply assume range 0-1024 (FD_SETSIZE)
+    $fds = @\scandir($path);
+
+    return $fds !== false ? $fds : \range(0, 1024);
+}
+
+/**
+ * Creates a Process with the given command modified in such a way that any additional FDs are explicitly not passed along
+ *
+ * @param string $command
+ * @return Process
+ * @internal
+ */
+function processWithoutFds($command)
+{
+    // try to get list of all open FDs
+    $fds = fds();
+
+    // do not inherit open FDs by explicitly closing all of them
+    foreach ($fds as $fd) {
+        if ($fd > 2) {
+            $command .= ' ' . $fd . '>&-';
+        }
+    }
+
+    // default `sh` only accepts single-digit FDs, so run in bash if needed
+    if ($fds && max($fds) > 9) {
+        $command = 'exec bash -c ' . escapeshellarg($command);
+    }
+
+    return new Process($command);
+}

src/SshProcessConnector.php

@@ -5,7 +5,6 @@
 use Clue\React\SshProxy\Io\CompositeConnection;
 use Clue\React\SshProxy\Io\LineSeparatedReader;
 use React\EventLoop\LoopInterface;
-use React\ChildProcess\Process;
 use React\Promise\Deferred;
 use React\Socket\ConnectorInterface;
 
@@ -67,26 +66,7 @@ public function connect($uri)
         }
 
         $command = $this->cmd . ' -W ' . \escapeshellarg($parts['host'] . ':' . $parts['port']);
-
-        // try to get list of all open FDs (Linux only) or simply assume range 3-1024 (FD_SETSIZE)
-        $fds = @scandir('/proc/self/fd');
-        if ($fds === false) {
-            $fds = range(3, 1024); // @codeCoverageIgnore
-        }
-
-        // do not inherit open FDs by explicitly closing all of them
-        foreach ($fds as $fd) {
-            if ($fd > 2) {
-                $command .= ' ' . $fd . '>&-';
-            }
-        }
-
-        // default `sh` only accepts single-digit FDs, so run in bash if needed
-        if ($fds && max($fds) > 9) {
-            $command = 'exec bash -c ' . escapeshellarg($command);
-        }
-
-        $process = new Process($command);
+        $process = Io\processWithoutFds($command);
         $process->start($this->loop);
 
         $deferred = new Deferred(function () use ($process, $uri) {

src/SshSocksConnector.php

@@ -195,27 +195,7 @@ public function idle()
 
     private function listen()
     {
-        $command = $this->cmd;
-
-        // try to get list of all open FDs (Linux only) or simply assume range 3-1024 (FD_SETSIZE)
-        $fds = @scandir('/proc/self/fd');
-        if ($fds === false) {
-            $fds = range(3, 1024); // @codeCoverageIgnore
-        }
-
-        // do not inherit open FDs by explicitly closing all of them
-        foreach ($fds as $fd) {
-            if ($fd > 2) {
-                $command .= ' ' . $fd . '>&-';
-            }
-        }
-
-        // default `sh` only accepts single-digit FDs, so run in bash if needed
-        if ($fds && max($fds) > 9) {
-            $command = 'exec bash -c ' . escapeshellarg($command);
-        }
-
-        $process = new Process($command);
+        $process = Io\processWithoutFds($this->cmd);
         $process->start($this->loop);
 
         $deferred = new Deferred(function () use ($process) {

tests/Io/FunctionsTest.php

@@ -0,0 +1,71 @@
+<?php
+
+use Clue\React\SshProxy\Io;
+use PHPUnit\Framework\TestCase;
+
+class FunctionsTest extends TestCase
+{
+    public function testFdsReturnsArray()
+    {
+        $fds = Io\fds();
+
+        $this->assertInternalType('array', $fds);
+    }
+
+    public function testFdsReturnsArrayWithStdioHandles()
+    {
+        // skip when running with closed handles: vendor/bin/phpunit 0<&-
+        if (!defined('STDIN') || !defined('STDOUT') || !defined('STDERR') || !@fstat(STDIN) || !@fstat(STDOUT) || !@fstat(STDERR)) {
+            $this->markTestSkipped('Test suite does not appear to run with standard I/O handles');
+        }
+
+        $fds = Io\fds();
+
+        $this->assertContains(0, $fds);
+        $this->assertContains(1, $fds);
+        $this->assertContains(2, $fds);
+    }
+
+    public function testFdsReturnsSameArrayTwice()
+    {
+        $fds = Io\fds();
+        $second = Io\fds();
+
+        $this->assertEquals($fds, $second);
+    }
+
+    public function testFdsWithInvalidPathReturnsArray()
+    {
+        $fds = Io\fds('/dev/null');
+
+        $this->assertInternalType('array', $fds);
+    }
+
+    public function testProcessWithoutFdsReturnsProcessWithoutClosingDefaultHandles()
+    {
+        $process = Io\processWithoutFds('sleep 10');
+
+        $this->assertInstanceOf('React\ChildProcess\Process', $process);
+
+        $this->assertNotContains('0>&-', $process->getCommand());
+        $this->assertNotContains('1>&-', $process->getCommand());
+        $this->assertNotContains('2>&-', $process->getCommand());
+    }
+
+    public function testProcessWithoutFdsReturnsProcessWithOriginalCommandPartOfActualCommandWhenDescriptorsNeedToBeClosed()
+    {
+        // skip when running with closed handles: vendor/bin/phpunit 0<&-
+        // bypass for example with dummy handles: vendor/bin/phpunit 8<&-
+        $fds = Io\fds();
+        if (!$fds || max($fds) < 3) {
+            $this->markTestSkipped('Did not detect additional file descriptors to be closed');
+        }
+
+        $process = Io\processWithoutFds('sleep 10');
+
+        $this->assertInstanceOf('React\ChildProcess\Process', $process);
+
+        $this->assertNotEquals('sleep 10', $process->getCommand());
+        $this->assertContains('sleep 10', $process->getCommand());
+    }
+}