From 0bb5e916d2ac983a8150d06ff10ac53e6bd552f6 Mon Sep 17 00:00:00 2001
From: "J. King" <jking@jkingweb.ca>
Date: Thu, 24 Jun 2021 18:32:20 -0400
Subject: [PATCH] Test PID writing

---
 lib/Service/Daemon.php             | 31 ++++++++++-------
 tests/cases/Service/TestDaemon.php | 56 +++++++++++++++++++++++++++++-
 2 files changed, 73 insertions(+), 14 deletions(-)

diff --git a/lib/Service/Daemon.php b/lib/Service/Daemon.php
index 6ed0da2..f5ae3d1 100644
--- a/lib/Service/Daemon.php
+++ b/lib/Service/Daemon.php
@@ -7,7 +7,7 @@ declare(strict_types=1);
 namespace JKingWeb\Arsse\Service;
 
 class Daemon {
-    protected const PID_PATTERN = '/^([1-9]\d{0,77})*$/D'; // no more than 78 digits (256-bit unsigned integer), starting with a digit other than zero
+    protected const PID_PATTERN = '/^([1-9]\d{0,77})?$/D'; // no more than 78 digits (256-bit unsigned integer), starting with a digit other than zero
 
     /** Daemonizes the process via the traditional sysvinit double-fork procedure
      * 
@@ -96,20 +96,25 @@ class Daemon {
     public function writePID(string $pidfile): void {
         if ($f = @fopen($pidfile, "c+")) {
             if (@flock($f, \LOCK_EX | \LOCK_NB)) {
-                // confirm that some other process didn't get in before us
-                $pid = fread($f, 80);
-                if (preg_match(static::PID_PATTERN, (string) $pid)) {
-                    if ($this->processExists((int) $pid)) {
-                        throw new Exception("pidDuplicate", ['pid' => $pid]);
+                try {
+                    // confirm that some other process didn't get in before us
+                    $pid = fread($f, 80);
+                    if (preg_match(static::PID_PATTERN, (string) $pid)) {
+                        if (strlen($pid) && $this->processExists((int) $pid)) {
+                            throw new Exception("pidDuplicate", ['pid' => $pid]);
+                        }
+                    } else {
+                        throw new Exception("pidCorrupt", ['pidfile' => $pidfile]);
                     }
-                } else {
-                    throw new Exception("pidCorrupt", ['pidfile' => $pidfile]);
+                    // write the PID to the pidfile
+                    rewind($f);
+                    if (!ftruncate($f, 0) || !fwrite($f, (string) posix_getpid())) {
+                        throw new Exception("pidInaccessible", ['pidfile' => $pidfile]);
+                    }
+                } finally {
+                    flock($f, \LOCK_UN);
+                    fclose($f);
                 }
-                // write the PID to the pidfile
-                rewind($f);
-                ftruncate($f, 0);
-                fwrite($f, (string) posix_getpid());
-                fclose($f);
             } else {
                 throw new Exception("pidLocked", ['pidfile' => $pidfile]);
             }
diff --git a/tests/cases/Service/TestDaemon.php b/tests/cases/Service/TestDaemon.php
index 7bf7e79..d35ef61 100644
--- a/tests/cases/Service/TestDaemon.php
+++ b/tests/cases/Service/TestDaemon.php
@@ -146,8 +146,62 @@ class TestDaemon extends \JKingWeb\Arsse\Test\AbstractTest {
             ["overlong",   new Exception("pidCorrupt")],
             ["unreadable", new Exception("pidInaccessible")],
             ["unwritable", null],
-            ["missing",    null],
             ["locked",     null],
+            ["missing",    null],
+            ["stale",      null],
+            ["empty",      null],
+        ];
+    }
+
+    /**
+     * @dataProvider providePidWriteChecks
+     * @requires extension posix
+     */
+    public function testCheckPidWrites(string $file, $exp) {
+        $pid = (string) posix_getpid();
+        $vfs = vfsStream::setup("pidtest", 0777, $this->pidfiles);
+        $path = $vfs->url()."/pid/";
+        // set up access blocks
+        $f = fopen($path."locked", "r+");
+        flock($f, \LOCK_EX | \LOCK_NB);
+        chmod($path."unreadable", 0333);
+        chmod($path."unwritable", 0555);
+        // set up mock daemon class
+        $this->daemon->processExists->with(2112)->returns(true);
+        $this->daemon->processExists->with(42)->returns(false);
+        $daemon = $this->daemon->get();
+        // perform the test
+        try {
+            if ($exp instanceof \Exception) {
+                $this->assertException($exp);
+                $exp = $this->pidfiles['pid'][$file] ?? false;
+                $daemon->writePID($path.$file);
+            } else {
+                $this->assertSame($exp, $daemon->writePID($path.$file));
+                $exp = $pid;
+            }
+        } finally {
+            flock($f, \LOCK_UN);
+            fclose($f);
+            chmod($path."unreadable", 0777);
+            $this->assertSame($exp, @file_get_contents($path.$file));
+        }
+    }
+
+    public function providePidWriteChecks(): iterable {
+        return [
+            ["current",    new Exception("pidDuplicate")],
+            ["malformed1", new Exception("pidCorrupt")],
+            ["malformed2", new Exception("pidCorrupt")],
+            ["malformed3", new Exception("pidCorrupt")],
+            ["bogus1",     new Exception("pidCorrupt")],
+            ["bogus2",     new Exception("pidCorrupt")],
+            ["bogus3",     new Exception("pidCorrupt")],
+            ["overlong",   new Exception("pidCorrupt")],
+            ["unreadable", new Exception("pidInaccessible")],
+            ["unwritable", new Exception("pidInaccessible")],
+            ["locked",     new Exception("pidLocked")],
+            ["missing",    null],
             ["stale",      null],
             ["empty",      null],
         ];