diff --git a/lib/AbstractException.php b/lib/AbstractException.php
index b0e275b..26f679e 100644
--- a/lib/AbstractException.php
+++ b/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.pidUnusable" => 10803,
- "Service/Exception.pidUnreadable" => 10804,
- "Service/Exception.pidUnwritable" => 10805,
- "Service/Exception.pidUncreatable" => 10806,
+ "Service/Exception.pidDirMissing" => 10802,
+ "Service/Exception.pidDirUnresolvable" => 10803,
+ "Service/Exception.pidUnusable" => 10804,
+ "Service/Exception.pidUnreadable" => 10805,
+ "Service/Exception.pidUnwritable" => 10806,
+ "Service/Exception.pidUncreatable" => 10807,
];
protected $symbol;
diff --git a/lib/CLI.php b/lib/CLI.php
index 5014b2c..c6e4969 100644
--- a/lib/CLI.php
+++ b/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
diff --git a/lib/Service/Daemon.php b/lib/Service/Daemon.php
index a7a0437..62fcf1c 100644
--- a/lib/Service/Daemon.php
+++ b/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) {
- return @realpath($path);
+ /** Resolves paths with relative components
+ *
+ * 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();
}
}
diff --git a/locale/en.php b/locale/en.php
index 1976399..57ea203 100644
--- a/locale/en.php
+++ b/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',
diff --git a/tests/cases/Service/TestDaemon.php b/tests/cases/Service/TestDaemon.php
new file mode 100644
index 0000000..5efdbe4
--- /dev/null
+++ b/tests/cases/Service/TestDaemon.php
@@ -0,0 +1,68 @@
+ [
+ '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"],
+ ];
+ }
+}
diff --git a/tests/cases/Service/TestPID.php b/tests/cases/Service/TestPID.php
deleted file mode 100644
index 77f2f5a..0000000
--- a/tests/cases/Service/TestPID.php
+++ /dev/null
@@ -1,59 +0,0 @@
- [
- '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")],
- ];
- }
-}
diff --git a/tests/phpunit.dist.xml b/tests/phpunit.dist.xml
index cbe6ea7..bd01e8f 100644
--- a/tests/phpunit.dist.xml
+++ b/tests/phpunit.dist.xml
@@ -142,7 +142,7 @@
cases/Service/TestService.php
cases/Service/TestSerial.php
cases/Service/TestSubprocess.php
- cases/Service/TestPID.php
+ cases/Service/TestDaemon.php
cases/CLI/TestCLI.php
cases/TestArsse.php