From: Dan Brown <redacted>
Date: Fri, 28 Apr 2023 17:36:40 +0000 (+0100)
Subject: Made a range of tweaks from testing
X-Git-Url: http://source.bookstackapp.com/system-cli/commitdiff_plain/e2f3bcf614ff3b84b309199a043a0a2fce046da5

Made a range of tweaks from testing

- Fixed some relative paths issues when ran in phar.
- Added helper class for doing path work.
- Updated composer runner timeout.
- Updated checks for paths more thorough.
- Removed default completion command.
---

diff --git a/src/Application.php b/src/Application.php
new file mode 100644
index 0000000..17dc197
--- /dev/null
+++ b/src/Application.php
@@ -0,0 +1,16 @@
+<?php
+declare(strict_types=1);
+
+namespace Cli;
+
+use Symfony\Component\Console\Application as BaseApplication;
+use Symfony\Component\Console\Command\HelpCommand;
+use Symfony\Component\Console\Command\ListCommand;
+
+class Application extends BaseApplication
+{
+    protected function getDefaultCommands(): array
+    {
+        return [new HelpCommand(), new ListCommand()];
+    }
+}
\ No newline at end of file
diff --git a/src/Commands/BackupCommand.php b/src/Commands/BackupCommand.php
index 915c582..a35ecb4 100644
--- a/src/Commands/BackupCommand.php
+++ b/src/Commands/BackupCommand.php
@@ -5,6 +5,7 @@ namespace Cli\Commands;
 use Cli\Services\AppLocator;
 use Cli\Services\EnvironmentLoader;
 use Cli\Services\MySqlRunner;
+use Cli\Services\Paths;
 use RecursiveDirectoryIterator;
 use SplFileInfo;
 use Symfony\Component\Console\Command\Command;
@@ -50,8 +51,8 @@ final class BackupCommand extends Command
         $zip->open($zipTempFile, ZipArchive::CREATE);
 
         // Add default files (.env config file and this CLI if existing)
-        $zip->addFile($appDir . DIRECTORY_SEPARATOR . '.env', '.env');
-        $cliPath = $appDir . DIRECTORY_SEPARATOR . 'bookstack-system-cli';
+        $zip->addFile(Paths::join($appDir, '.env'), '.env');
+        $cliPath = Paths::join($appDir, 'bookstack-system-cli');
         if (file_exists($cliPath)) {
             $zip->addFile($cliPath, 'bookstack-system-cli');
         }
@@ -70,7 +71,7 @@ final class BackupCommand extends Command
 
         if ($handleThemes) {
             $output->writeln("<info>Adding BookStack theme folders to backup archive...</info>");
-            $this->addFolderToZipRecursive($zip, implode(DIRECTORY_SEPARATOR, [$appDir, 'themes']), 'themes');
+            $this->addFolderToZipRecursive($zip, Paths::join($appDir, 'themes'), 'themes');
         }
 
         // Close off our zip and move it to the required location
@@ -103,11 +104,13 @@ final class BackupCommand extends Command
     /**
      * Build a full zip path from the given suggestion, which may be empty,
      * a path to a folder, or a path to a file in relative or absolute form.
+     * Targets the <app>/backups directory by default if existing, otherwise <app>.
      * @throws CommandError
      */
     protected function buildZipFilePath(string $suggestedOutPath, string $appDir): string
     {
-        $zipDir = getcwd() ?: $appDir;
+        $suggestedOutPath = Paths::resolve($suggestedOutPath);
+        $zipDir = Paths::join($appDir, 'backups');
         $zipName = "bookstack-backup-" . date('Y-m-d-His') . '.zip';
 
         if ($suggestedOutPath) {
@@ -119,12 +122,20 @@ final class BackupCommand extends Command
             } else {
                 throw new CommandError("Could not resolve provided [{$suggestedOutPath}] path to an existing folder.");
             }
+        } else {
+            if (!is_dir($zipDir)) {
+                $zipDir = $appDir;
+            }
         }
 
-        $fullPath = $zipDir . DIRECTORY_SEPARATOR . $zipName;
+        $fullPath = Paths::join($zipDir, $zipName);
 
         if (file_exists($fullPath)) {
             throw new CommandError("Target ZIP output location at [{$fullPath}] already exists.");
+        } else if (!is_dir($zipDir)) {
+            throw new CommandError("Target ZIP output directory [{$fullPath}] could not be found.");
+        } else if (!is_writable($zipDir)) {
+            throw new CommandError("Target ZIP output directory [{$fullPath}] is not writable.");
         }
 
         return $fullPath;
@@ -136,8 +147,8 @@ final class BackupCommand extends Command
      */
     protected function addUploadFoldersToZip(ZipArchive $zip, string $appDir): void
     {
-        $this->addFolderToZipRecursive($zip, implode(DIRECTORY_SEPARATOR, [$appDir, 'public', 'uploads']), 'public/uploads');
-        $this->addFolderToZipRecursive($zip, implode(DIRECTORY_SEPARATOR, [$appDir, 'storage', 'uploads']), 'storage/uploads');
+        $this->addFolderToZipRecursive($zip, Paths::join($appDir, 'public', 'uploads'), 'public/uploads');
+        $this->addFolderToZipRecursive($zip, Paths::join($appDir, 'storage', 'uploads'), 'storage/uploads');
     }
 
     /**
diff --git a/src/Commands/InitCommand.php b/src/Commands/InitCommand.php
index e35e7bb..ef79273 100644
--- a/src/Commands/InitCommand.php
+++ b/src/Commands/InitCommand.php
@@ -4,6 +4,7 @@ namespace Cli\Commands;
 
 use Cli\Services\ComposerLocator;
 use Cli\Services\EnvironmentLoader;
+use Cli\Services\Paths;
 use Cli\Services\ProgramRunner;
 use Cli\Services\RequirementsValidator;
 use Symfony\Component\Console\Command\Command;
@@ -49,7 +50,7 @@ class InitCommand extends Command
         $this->installComposerDependencies($composer, $installDir);
 
         $output->writeln("<info>Creating .env file from .env.example...</info>");
-        copy($installDir . DIRECTORY_SEPARATOR . '.env.example', $installDir . DIRECTORY_SEPARATOR . '.env');
+        copy(Paths::join($installDir, '.env.example'), Paths::join($installDir, '.env'));
         sleep(1);
 
         $output->writeln("<info>Generating app key...</info>");
@@ -73,7 +74,7 @@ class InitCommand extends Command
             ->withIdleTimeout(5)
             ->withEnvironment(EnvironmentLoader::load($installDir))
             ->runCapturingAllOutput([
-                $installDir . DIRECTORY_SEPARATOR . 'artisan',
+                Paths::join($installDir, 'artisan'),
                 'key:generate', '--force', '-n', '-q'
             ]);
 
@@ -150,22 +151,20 @@ class InitCommand extends Command
      */
     protected function getInstallDir(string $suggestedDir): string
     {
-        $dir = getcwd();
-
-        if ($suggestedDir) {
-            if (is_file($suggestedDir)) {
-                throw new CommandError("Was provided [{$suggestedDir}] as an install path but existing file provided.");
-            } else if (is_dir($suggestedDir)) {
-                $dir = realpath($suggestedDir);
-            } else if (is_dir(dirname($suggestedDir))) {
-                $created = mkdir($suggestedDir);
-                if (!$created) {
-                    throw new CommandError("Could not create directory [{$suggestedDir}] for install.");
-                }
-                $dir = realpath($suggestedDir);
-            } else {
-                throw new CommandError("Could not resolve provided [{$suggestedDir}] path to an existing folder.");
+        $dir = Paths::resolve($suggestedDir);
+
+        if (is_file($dir)) {
+            throw new CommandError("Was provided [{$dir}] as an install path but existing file provided.");
+        } else if (is_dir($dir) && realpath($dir)) {
+            $dir = realpath($dir);
+        } else if (is_dir(dirname($dir))) {
+            $created = mkdir($dir);
+            if (!$created) {
+                throw new CommandError("Could not create directory [{$dir}] for install.");
             }
+            $dir = realpath($dir);
+        } else {
+            throw new CommandError("Could not resolve provided [{$dir}] path to an existing folder.");
         }
 
         return $dir;
diff --git a/src/Commands/RestoreCommand.php b/src/Commands/RestoreCommand.php
index b38589c..2f71db0 100644
--- a/src/Commands/RestoreCommand.php
+++ b/src/Commands/RestoreCommand.php
@@ -66,7 +66,7 @@ class RestoreCommand extends Command
         }
 
         $output->writeln("<info>The checked elements will be restored into [{$appDir}].</info>");
-        $output->writeln("<info>Existing content may be overwritten.</info>");
+        $output->writeln("<info>Existing content will be overwritten.</info>");
 
         if (!$interactions->confirm("Do you want to continue?")) {
             $output->writeln("<info>Stopping restore operation.</info>");
diff --git a/src/Services/AppLocator.php b/src/Services/AppLocator.php
index b6710a8..828bb22 100644
--- a/src/Services/AppLocator.php
+++ b/src/Services/AppLocator.php
@@ -8,7 +8,7 @@ class AppLocator
 {
     public static function search(string $directory = ''): string
     {
-        $directoriesToSearch = $directory ? [$directory] : [
+        $directoriesToSearch = $directory ? [Paths::resolve($directory)] : [
             getcwd(),
             static::getCliDirectory(),
         ];
diff --git a/src/Services/ComposerLocator.php b/src/Services/ComposerLocator.php
index db46d9f..f29c588 100644
--- a/src/Services/ComposerLocator.php
+++ b/src/Services/ComposerLocator.php
@@ -15,7 +15,7 @@ class ComposerLocator
     {
         return (new ProgramRunner('composer', '/usr/local/bin/composer'))
             ->withTimeout(300)
-            ->withIdleTimeout(15)
+            ->withIdleTimeout(60)
             ->withAdditionalPathLocation($this->appDir);
     }
 
diff --git a/src/Services/Paths.php b/src/Services/Paths.php
new file mode 100644
index 0000000..d0bb10f
--- /dev/null
+++ b/src/Services/Paths.php
@@ -0,0 +1,63 @@
+<?php declare(strict_types=1);
+
+namespace Cli\Services;
+
+class Paths
+{
+
+    /**
+     * Join together the given path components.
+     * Does no resolving or cleaning.
+     * Only the $base will remain absolute if so,
+     * $parts are assumed to treated as non-absolute paths.
+     */
+    public static function join(string $base, string ...$parts): string
+    {
+        $outParts = [rtrim($base, '/\\')];
+        foreach ($parts as $part) {
+            $outParts[] = trim($part, '/\\');
+        }
+
+        return implode(DIRECTORY_SEPARATOR, $outParts);
+    }
+
+    /**
+     * Resolve the full path for the given path/sub-path.
+     * If the provided path is not absolute, it will be returned
+     * be resolved to the provided $base or current working directory if
+     * no $base is provided.
+     */
+    public static function resolve(string $path, string $base = ''): string
+    {
+        if (str_starts_with($path, '/') || str_starts_with($path, '\\')) {
+            return DIRECTORY_SEPARATOR . self::clean($path);
+        }
+
+        $base = rtrim($base ?: getcwd(), '/');
+        $joined = $base . '/' . $path;
+        $absoluteBase = (str_starts_with($base, '/') || str_starts_with($base, '\\'));
+        return ($absoluteBase ? '/' : '') . self::clean($joined);
+    }
+
+    /**
+     * Clean the given path so that all up/down navigations are resolved,
+     * and so its using system-correct directory separators.
+     * Credit to Sven Arduwie in PHP docs:
+     * https://www.php.net/manual/en/function.realpath.php#84012
+     */
+    private static function clean(string $path): string
+    {
+        $path = str_replace(['/', '\\'], DIRECTORY_SEPARATOR, $path);
+        $parts = array_filter(explode(DIRECTORY_SEPARATOR, $path), 'strlen');
+        $absolutes = [];
+        foreach ($parts as $part) {
+            if ('.' == $part) continue;
+            if ('..' == $part) {
+                array_pop($absolutes);
+            } else {
+                $absolutes[] = $part;
+            }
+        }
+        return implode(DIRECTORY_SEPARATOR, $absolutes);
+    }
+}
\ No newline at end of file
diff --git a/src/Services/RequirementsValidator.php b/src/Services/RequirementsValidator.php
index 008bb4e..b057410 100644
--- a/src/Services/RequirementsValidator.php
+++ b/src/Services/RequirementsValidator.php
@@ -18,7 +18,7 @@ class RequirementsValidator
             $errors[] = "PHP >= 8.0.2 is required to install BookStack.";
         }
 
-        $requiredExtensions = ['bcmath', 'curl', 'gd', 'iconv', 'libxml', 'mbstring', 'mysqlnd', 'xml'];
+        $requiredExtensions = ['curl', 'gd', 'iconv', 'libxml', 'mbstring', 'mysqlnd', 'xml'];
         foreach ($requiredExtensions as $extension) {
             if (!extension_loaded($extension)) {
                 $errors[] = "The \"{$extension}\" PHP extension is required by not active.";
diff --git a/src/app.php b/src/app.php
index 0ff90cd..e4cf57b 100644
--- a/src/app.php
+++ b/src/app.php
@@ -1,10 +1,11 @@
 <?php
+declare(strict_types=1);
 
+use Cli\Application;
 use Cli\Commands\BackupCommand;
 use Cli\Commands\InitCommand;
 use Cli\Commands\RestoreCommand;
 use Cli\Commands\UpdateCommand;
-use Symfony\Component\Console\Application;
 
 // Setup our CLI
 $app = new Application('bookstack-system');
@@ -15,5 +16,4 @@ $app->add(new UpdateCommand());
 $app->add(new InitCommand());
 $app->add(new RestoreCommand());
 
-
-return $app;
\ No newline at end of file
+return $app;
diff --git a/tests/PathTest.php b/tests/PathTest.php
new file mode 100644
index 0000000..42eb571
--- /dev/null
+++ b/tests/PathTest.php
@@ -0,0 +1,36 @@
+<?php
+
+namespace Tests;
+
+use Cli\Services\Paths;
+
+class PathTest extends TestCase
+{
+
+    public function test_resolve()
+    {
+        $cwd = getcwd();
+        $this->assertEquals('/my/path', Paths::resolve('/my/path/'));
+        $this->assertEquals('/my/path', Paths::resolve('\\my\\path'));
+        $this->assertEquals('/my/path', Paths::resolve('/my/path'));
+        $this->assertEquals('/my/path', Paths::resolve('/my/cats/../path'));
+        $this->assertEquals('/my/path', Paths::resolve('/my/cats/.././path'));
+        $this->assertEquals('/my/path', Paths::resolve('/my/path', '/root'));
+        $this->assertEquals('/root/my/path', Paths::resolve('my/path', '/root'));
+        $this->assertEquals('/my/path', Paths::resolve('../my/path', '/root'));
+        $this->assertEquals("{$cwd}/my/path", Paths::resolve('my/path'));
+        $this->assertEquals("{$cwd}", Paths::resolve(''));
+    }
+
+    public function test_join()
+    {
+        $this->assertEquals('/my/path', Paths::join('/my/', 'path'));
+        $this->assertEquals('/my/path', Paths::join('/my/', '/path'));
+        $this->assertEquals('/my/path', Paths::join('/my', 'path'));
+        $this->assertEquals('/my/path', Paths::join('/my', 'path/'));
+        $this->assertEquals('my/path/to/here', Paths::join('my', 'path', 'to', 'here'));
+        $this->assertEquals('my', Paths::join('my'));
+        $this->assertEquals('my/path', Paths::join('my//', '//path//'));
+        $this->assertEquals('/my/path', Paths::join('/my//', '\\path\\'));
+    }
+}
\ No newline at end of file