From eee0ec80daa8c88f467a80e9fa811bcfc19dfa00 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20L=C3=BCck?= <christian@lueck.tv>
Date: Tue, 22 Jan 2019 20:53:12 +0100
Subject: [PATCH] Do not inherit open FDs to SQLite worker by overwriting and
 closing

This somewhat obscure PR ensures that we do not inherit open file
descriptors (FDs) to the SQLite child worker process. This can cause all
sorts of errors in long running applications and really is not desired
here.

This is implemented by explicitly overwriting all superfluous FDs with
dummy file handles and then closing all of these in the implicit `sh`
child process before launching the actual php binary. PHP does not
support `FD_CLOEXEC`, `O_CLOEXEC` or `SOCK_CLOEXEC`  and this appears to
be the best work around I could find (yes, I should probably write a
lengthy, somewhat technical blog post about this). Additionally, this PR
includes a test to verify this works on all supported platforms and this
could perhaps be used as a starting point for other libraries (YMMV).

This builds on top of https://github.com/clue/reactphp-ssh-proxy/pull/2
and https://github.com/clue/reactphp-ssh-proxy/pull/10
---
 src/Database.php                 | 44 ++++++++++++++++++++++++++-
 tests/FunctionalDatabaseTest.php | 51 ++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/src/Database.php b/src/Database.php
index d1235af..f6ebf76 100644
--- a/src/Database.php
+++ b/src/Database.php
@@ -83,7 +83,49 @@ class Database extends EventEmitter
      */
     public static function open(LoopInterface $loop, $filename, $flags = null)
     {
-        $process = new Process('exec ' . \escapeshellarg(\PHP_BINARY) . ' ' . \escapeshellarg(__DIR__ . '/../res/sqlite-worker.php'));
+        $command = 'exec ' . \escapeshellarg(\PHP_BINARY) . ' ' . \escapeshellarg(__DIR__ . '/../res/sqlite-worker.php');
+
+        // Try to get list of all open FDs (Linux/Mac and others)
+        $fds = @\scandir('/dev/fd');
+
+        // Otherwise try temporarily duplicating file descriptors in the range 0-1024 (FD_SETSIZE).
+        // This is known to work on more exotic platforms and also inside chroot
+        // environments without /dev/fd. Causes many syscalls, but still rather fast.
+        // @codeCoverageIgnoreStart
+        if ($fds === false) {
+            $fds = array();
+            for ($i = 0; $i <= 1024; ++$i) {
+                $copy = @\fopen('php://fd/' . $i, 'r');
+                if ($copy !== false) {
+                    $fds[] = $i;
+                    \fclose($copy);
+                }
+            }
+        }
+        // @codeCoverageIgnoreEnd
+
+        // launch process with default STDIO pipes
+        $pipes = array(
+            array('pipe', 'r'),
+            array('pipe', 'w'),
+            array('pipe', 'w')
+        );
+
+        // 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 . '>&-';
+            }
+        }
+
+        // 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, null, null, $pipes);
         $process->start($loop);
 
         $db = new Database($process);
diff --git a/tests/FunctionalDatabaseTest.php b/tests/FunctionalDatabaseTest.php
index 431951e..448116e 100644
--- a/tests/FunctionalDatabaseTest.php
+++ b/tests/FunctionalDatabaseTest.php
@@ -41,6 +41,33 @@ public function testOpenMemoryDatabaseResolvesWithDatabaseAndRunsUntilQuit()
         $loop->run();
     }
 
+    public function testOpenMemoryDatabaseShouldNotInheritActiveFileDescriptors()
+    {
+        $server = stream_socket_server('tcp://127.0.0.1:0');
+        $address = stream_socket_get_name($server, false);
+
+        if (@stream_socket_server('tcp://' . $address) !== false) {
+            $this->markTestSkipped('Platform does not prevent binding to same address (Windows?)');
+        }
+
+        $loop = Factory::create();
+
+        $promise = Database::open($loop, ':memory:');
+
+        // close server and ensure we can start a new server on the previous address
+        // the pending SQLite process should not inherit the existing server socket
+        fclose($server);
+        $server = stream_socket_server('tcp://' . $address);
+        $this->assertTrue(is_resource($server));
+        fclose($server);
+
+        $promise->then(function (Database $db) {
+            $db->close();
+        });
+
+        $loop->run();
+    }
+
     public function testOpenInvalidPathRejects()
     {
         $loop = Factory::create();
@@ -83,6 +110,30 @@ public function testQuitResolvesAndRunsUntilQuit()
         $loop->run();
     }
 
+
+    public function testQuitResolvesAndRunsUntilQuitWhenParentHasManyFileDescriptors()
+    {
+        $servers = array();
+        for ($i = 0; $i < 100; ++$i) {
+            $servers[] = stream_socket_server('tcp://127.0.0.1:0');
+        }
+
+        $loop = Factory::create();
+
+        $promise = Database::open($loop, ':memory:');
+
+        $once = $this->expectCallableOnce();
+        $promise->then(function (Database $db) use ($once){
+            $db->quit()->then($once);
+        });
+
+        $loop->run();
+
+        foreach ($servers as $server) {
+            fclose($server);
+        }
+    }
+
     public function testQuitTwiceWillRejectSecondCall()
     {
         $loop = Factory::create();