Do not inherit open FDs to SSH child process by overwriting and closing

This fixes a possible race condition where open FDs where in fact
inherited to the wrapping shell before it had a chance to close them
again when it is being replaced with the actual SSH binary. This builds
on top of reactphp/child-process#65

Author: clue

composer.json

@@ -17,7 +17,7 @@
     "require": {
         "php": ">=5.3",
         "clue/socks-react": "^1.0",
-        "react/child-process": "^0.5",
+        "react/child-process": "^0.6",
         "react/event-loop": "^1.0 || ^0.5",
         "react/promise": "^2.1 || ^1.2.1",
         "react/socket": "^1.1",

src/Io/functions.php

@@ -42,12 +42,21 @@ function fds($path = '/dev/fd')
  */
 function processWithoutFds($command)
 {
+    // launch process with default STDIO pipes
+    $pipes = array(
+        array('pipe', 'r'),
+        array('pipe', 'w'),
+        array('pipe', 'w')
+    );
+
     // try to get list of all open FDs
     $fds = fds();
 
-    // do not inherit open FDs by explicitly closing all of them
+    // do not inherit open FDs by explicitly overwriting existing FDs with dummy files
+    // additionally, close all dummy files in the child process again
     foreach ($fds as $fd) {
         if ($fd > 2) {
+            $pipes[$fd] = array('file', '/dev/null', 'r');
             $command .= ' ' . $fd . '>&-';
         }
     }
@@ -57,5 +66,5 @@ function processWithoutFds($command)
         $command = 'exec bash -c ' . escapeshellarg($command);
     }
 
-    return new Process($command);
+    return new Process($command, null, null, $pipes);
 }

tests/FunctionalSshProcessConnectorTest.php

@@ -69,7 +69,7 @@ public function testConnectValidTargetWillReturnPromiseWhichResolvesToConnection
         $connection->close();
     }
 
-    public function testConnectValidTargetWillNotInheritActiveFileDescriptors()
+    public function testConnectPendingWillNotInheritActiveFileDescriptors()
     {
         $server = stream_socket_server('tcp://127.0.0.1:0');
         $address = stream_socket_get_name($server, false);
@@ -83,17 +83,15 @@ public function testConnectValidTargetWillNotInheritActiveFileDescriptors()
             $this->markTestSkipped('Platform does not prevent binding to same address (Windows?)');
         }
 
-        // wait for successful connection
         $promise = $this->connector->connect('example.com:80');
-        $connection = \Clue\React\Block\await($promise, $this->loop, self::TIMEOUT);
 
         // close server and ensure we can start a new server on the previous address
-        // the open SSH connection process should not inherit the existing server socket
+        // the pending SSH connection process should not inherit the existing server socket
         fclose($server);
         $server = stream_socket_server('tcp://' . $address);
         $this->assertTrue(is_resource($server));
-
         fclose($server);
-        $connection->close();
+
+        $promise->cancel();
     }
 }

tests/FunctionalSshSocksConnectorTest.php

@@ -89,7 +89,7 @@ public function testConnectValidTargetWillReturnPromiseWhichResolvesToConnection
         $connection->close();
     }
 
-    public function testConnectValidTargetWillNotInheritActiveFileDescriptors()
+    public function testConnectPendingWillNotInheritActiveFileDescriptors()
     {
         $server = stream_socket_server('tcp://127.0.0.1:0');
         $address = stream_socket_get_name($server, false);
@@ -103,17 +103,15 @@ public function testConnectValidTargetWillNotInheritActiveFileDescriptors()
             $this->markTestSkipped('Platform does not prevent binding to same address (Windows?)');
         }
 
-        // wait for successful connection
         $promise = $this->connector->connect('example.com:80');
-        $connection = \Clue\React\Block\await($promise, $this->loop, self::TIMEOUT);
 
         // close server and ensure we can start a new server on the previous address
-        // the open SSH connection process should not inherit the existing server socket
+        // the pending SSH connection process should not inherit the existing server socket
         fclose($server);
         $server = stream_socket_server('tcp://' . $address);
         $this->assertTrue(is_resource($server));
-
         fclose($server);
-        $connection->close();
+
+        $promise->cancel();
     }
 }