Finish testing PID file path checking

lib/AbstractException.php

@@ -103,11 +103,12 @@ abstract class AbstractException extends \Exception {
         "ImportExport/Exception.invalidTagName"       => 10615,
         "Rule/Exception.invalidPattern"               => 10701,
         "Service/Exception.pidNotFile"                => 10801,
-        "Service/Exception.pidDirNotFound"            => 10802,
+        "Service/Exception.pidDirMissing"             => 10802,
-        "Service/Exception.pidUnusable"               => 10803,
+        "Service/Exception.pidDirUnresolvable"        => 10803,
-        "Service/Exception.pidUnreadable"             => 10804,
+        "Service/Exception.pidUnusable"               => 10804,
-        "Service/Exception.pidUnwritable"             => 10805,
+        "Service/Exception.pidUnreadable"             => 10805,
-        "Service/Exception.pidUncreatable"            => 10806,
+        "Service/Exception.pidUnwritable"             => 10806,
+        "Service/Exception.pidUncreatable"            => 10807,
     ];
 
     protected $symbol;

lib/CLI.php

@@ -183,7 +183,7 @@ USAGE_TEXT;
         // create a Daemon object which contains various helper functions
         $daemon = Arsse::$obj->get(Daemon::class);
         // resolve the PID file to its absolute path; this also checks its readability and writability
-        $pidfile = $daemon->resolvePID($pidfile);
+        $pidfile = $daemon->checkPIDFilePath($pidfile);
         // daemonize
         $daemon->fork($pidfile);
         // start the fetching service as normal

lib/Service/Daemon.php

@@ -109,33 +109,74 @@ class Daemon {
     }
 
     /** Resolves the PID file path and ensures the file or parent directory is writable */
-    public function resolvePID(string $pidfile): string {
+    public function checkPIDFilePath(string $pidfile): string {
         $dir = dirname($pidfile);
         $file = basename($pidfile);
+        $base = $this->resolveRelativePath($dir);
         if (!strlen($file)) {
             throw new Exception("pidNotFile", ['pidfile' => $dir]);
-        } elseif ($base = @$this->realpath($dir)) {
+        } elseif ($base) {
             $out = "$base/$file";
             if (file_exists($out)) {
-                if (!is_readable($out) && !is_writable($out)) {
+                if (!is_file($out)) {
+                    throw new Exception("pidNotFile", ['pidfile' => $out]);
+                } elseif (!is_readable($out) && !is_writable($out)) {
                     throw new Exception("pidUnusable", ['pidfile' => $out]);
                 } elseif (!is_readable($out)) {
-                    throw new Exception("pidunreadable", ['pidfile' => $out]);
+                    throw new Exception("pidUnreadable", ['pidfile' => $out]);
                 } elseif (!is_writable($out)) {
                     throw new Exception("pidUnwritable", ['pidfile' => $out]);
-                } elseif (!is_file($out)) {
-                    throw new Exception("pidNotFile", ['pidfile' => $out]);
                 }
+            } elseif (!is_dir($base)) {
+                throw new Exception("pidDirMissing", ['piddir' => $dir]);
             } elseif (!is_writable($base)) {
                 throw new Exception("pidUncreatable", ['pidfile' => $out]);
             }
         } else {
-            throw new Exception("pidDirNotFound", ['piddir' => $dir]);
+            throw new Exception("pidDirUnresolvable", ['piddir' => $dir]);
         }
         return $out;
     }
 
-    protected function realpath(string $path) {
+    /** Resolves paths with relative components
-        return @realpath($path);
+     * 
+     * This method has fewer filesystem access requirements than the native
+     * realpath() function. The current working directory most be resolvable
+     * for a relative path, but for absolute paths with relativelu components
+     * the filesystem is not involved at all.
+     * 
+     * Consequently symbolic links are not resolved.
+     * 
+     * @return string|false
+     */
+    public function resolveRelativePath(string $path) {
+        if ($path[0] !== "/") {
+            $cwd = $this->cwd();
+            if ($cwd === false) {
+                return false;
+            }
+            $path = substr($cwd, 1)."/".$path;
+        }
+        $path = explode("/", substr($path, 1));
+        $out = [];
+        foreach ($path as $p) {
+            if ($p === "..") {
+                array_pop($out);
+            } elseif ($p === ".") {
+                continue;
+            } else {
+                $out[] = $p;
+            }
+        }
+        return "/".implode("/", $out);
+    }
+
+    /** Wrapper around posix_getcwd to facilitate testing
+     * 
+     * @return string|false
+     * @codeCoverageIgnore
+     */
+    protected function cwd() {
+        return posix_getcwd();
     }
 }

locale/en.php

@@ -209,7 +209,8 @@ return [
     'Exception.JKingWeb/Arsse/ImportExport/Exception.invalidTagName'       => 'Input data contains an invalid tag name',
     'Exception.JKingWeb/Arsse/Rule/Exception.invalidPattern'               => 'Specified rule pattern is invalid',
     'Exception.JKingWeb/Arsse/Service/Exception.pidNotFile'                => 'Specified PID file location "{pidfile}" must be a regular file',
-    'Exception.JKingWeb/Arsse/Service/Exception.pidDirNotFound'            => 'Parent directory "{piddir}" of PID file does not exist',
+    'Exception.JKingWeb/Arsse/Service/Exception.pidDirMissing'             => 'Parent directory "{piddir}" of PID file does not exist',
+    'Exception.JKingWeb/Arsse/Service/Exception.pidDirUnresolvable'        => 'Parent directory "{piddir}" of PID file could not be resolved to its absolute path',
     'Exception.JKingWeb/Arsse/Service/Exception.pidUnreadable'             => 'Insufficient permissions to open PID file "{pidfile}" for reading',
     'Exception.JKingWeb/Arsse/Service/Exception.pidUnwritable'             => 'Insufficient permissions to open PID file "{pidfile}" for writing',
     'Exception.JKingWeb/Arsse/Service/Exception.pidUnusable'               => 'Insufficient permissions to open PID file "{pidfile}" for reading or writing',

tests/cases/Service/TestDaemon.php

@@ -0,0 +1,68 @@
+<?php
+/** @license MIT
+ * Copyright 2017 J. King, Dustin Wilson et al.
+ * See LICENSE and AUTHORS files for details */
+
+declare(strict_types=1);
+namespace JKingWeb\Arsse\TestCase\Service;
+
+use JKingWeb\Arsse\Service\Daemon;
+use JKingWeb\Arsse\Service\Exception;
+use org\bovigo\vfs\vfsStream;
+
+/** @covers \JKingWeb\Arsse\Service\Daemon */
+class TestDaemon extends \JKingWeb\Arsse\Test\AbstractTest {
+    protected $pidfiles = [
+        'errors' => [
+            'create'    => [],
+            'read'      => "cannot be read",
+            'write'     => "cannot be written to",
+            'readwrite' => "can neither be read nor written to",
+        ],
+        'ok' => [
+            'dir' => [],
+            'file' => "this file can be fully accessed",
+        ],
+    ];
+
+    public function setUp(): void {
+        parent::setUp();
+        $this->daemon = $this->partialMock(Daemon::class);
+    }
+
+    /** @dataProvider providePidChecks */
+    public function testCheckPidFiles(string $file, bool $accessible, $exp): void {
+        $vfs = vfsStream::setup("pidtest", 0777, $this->pidfiles);
+        $path = $vfs->url()."/";
+        // set up access blocks
+        chmod($path."errors/create", 0555);
+        chmod($path."errors/read", 0333);
+        chmod($path."errors/write", 0555);
+        chmod($path."errors/readwrite", 0111);
+        // set up mock daemon class
+        $this->daemon->resolveRelativePath->returns($accessible ? dirname($path.$file) : false);
+        $daemon = $this->daemon->get();
+        // perform the test
+        if ($exp instanceof \Exception) {
+            $this->assertException($exp);
+            $daemon->checkPIDFilePath($file);
+        } else {
+            $this->assertSame($path.$exp, $daemon->checkPIDFilePath($file));
+        }
+    }
+
+    public function providePidChecks(): iterable {
+        return [
+            ["ok/file",           false, new Exception("pidDirUnresolvable")],
+            ["not/found",         true,  new Exception("pidDirMissing")],
+            ["errors/create/pid", true,  new Exception("pidUncreatable")],
+            ["errors/read",       true,  new Exception("pidUnreadable")],
+            ["errors/write",      true,  new Exception("pidUnwritable")],
+            ["errors/readwrite",  true,  new Exception("pidUnusable")],
+            ["",                  true,  new Exception("pidNotFile")],
+            ["ok/dir",            true,  new Exception("pidNotFile")],
+            ["ok/file",           true,  "ok/file"],
+            ["ok/dir/file",       true,  "ok/dir/file"],
+        ];
+    }
+}

tests/cases/Service/TestPID.php

@@ -1,59 +0,0 @@
-<?php
-/** @license MIT
- * Copyright 2017 J. King, Dustin Wilson et al.
- * See LICENSE and AUTHORS files for details */
-
-declare(strict_types=1);
-namespace JKingWeb\Arsse\TestCase\Service;
-
-use JKingWeb\Arsse\Service\Daemon;
-use JKingWeb\Arsse\Service\Exception;
-use org\bovigo\vfs\vfsStream;
-
-/** @covers \JKingWeb\Arsse\Service */
-class TestPID extends \JKingWeb\Arsse\Test\AbstractTest {
-    protected $pidfiles = [
-        'errors' => [
-            'create'    => [],
-            'read'      => "",
-            'write'     => "",
-            'readwrite' => "",
-        ],
-        'ok' => [
-            'dir' => [],
-            'file' => "",
-        ],
-    ];
-
-    public function setUp(): void {
-        parent::setUp();
-        $this->daemon = $this->partialMock(Daemon::class);
-    }
-
-    /** @dataProvider providePidResolutions */
-    public function testResolvePidFiles(string $file, bool $realpath, $exp): void {
-        $vfs = vfsStream::setup("pidtest", 0777, $this->pidfiles);
-        $path = $vfs->url()."/";
-        // set up access blocks
-        chmod($path."errors/create", 0555);
-        chmod($path."errors/read", 0333);
-        chmod($path."errors/write", 0555);
-        chmod($path."errors/readwrite", 0111);
-        // set up mock daemon class
-        $this->daemon->realPath->returns($realpath ? $path.$file : false);
-        $daemon = $this->daemon->get();
-        // perform the test
-        if ($exp instanceof \Exception) {
-            $this->assertException($exp);
-            $daemon->resolvePID($file);
-        } else {
-            $this->assertSame($exp, $daemon->resolvePID($file));
-        }
-    }
-
-    public function providePidResolutions(): iterable {
-        return [
-            ["errors/create", true, new Exception("pidUncreatable")],
-        ];
-    }
-}

tests/phpunit.dist.xml

@@ -142,7 +142,7 @@
         <file>cases/Service/TestService.php</file>
         <file>cases/Service/TestSerial.php</file>
         <file>cases/Service/TestSubprocess.php</file>
-        <file>cases/Service/TestPID.php</file>
+        <file>cases/Service/TestDaemon.php</file>
         <file>cases/CLI/TestCLI.php</file>
         <file>cases/TestArsse.php</file>
     </testsuite>