From: Dan Brown <redacted>
Date: Fri, 3 Mar 2023 21:01:30 +0000 (+0000)
Subject: Added error handling and validation to backup command
X-Git-Url: http://source.bookstackapp.com/system-cli/commitdiff_plain/e6606fb9474581a4f243106c272ea5e9364f4e21

Added error handling and validation to backup command
---

diff --git a/scripts/Commands/BackupCommand.php b/scripts/Commands/BackupCommand.php
index cf2ca35..e6cbc90 100644
--- a/scripts/Commands/BackupCommand.php
+++ b/scripts/Commands/BackupCommand.php
@@ -5,6 +5,7 @@ namespace Cli\Commands;
 use Minicli\Command\CommandCall;
 use RecursiveDirectoryIterator;
 use SplFileInfo;
+use Symfony\Component\Process\Exception\ProcessTimedOutException;
 use Symfony\Component\Process\ExecutableFinder;
 use Symfony\Component\Process\Process;
 use ZipArchive;
@@ -16,49 +17,74 @@ final class BackupCommand
     ) {
     }
 
+    /**
+     * @throws CommandError
+     */
     public function handle(CommandCall $input)
     {
+        $this->ensureRequiredExtensionInstalled();
+
         $handleDatabase = !$input->hasFlag('no-database');
         $handleUploads = !$input->hasFlag('no-uploads');
-        $suggestedOutPath = $input->subcommand ?: '';
-
-        // TODO - Validate DB vars
-        // TODO - Backup themes directory, extra flag for no-themes
-        // TODO - Backup the running phar? For easier direct restore...
-        // TODO - Error handle each stage
-        // TODO - Validate zip (and any other extensions required) are active.
+        $handleThemes = !$input->hasFlag('no-themes');
+        $suggestedOutPath = $input->subcommand;
+        if ($suggestedOutPath === 'default') {
+            $suggestedOutPath = '';
+        }
 
         $zipOutFile = $this->buildZipFilePath($suggestedOutPath);
-        $dumpTempFile = $this->createDatabaseDump();
 
         // Create a new ZIP file
         $zipTempFile = tempnam(sys_get_temp_dir(), 'bsbackup');
         $zip = new ZipArchive();
         $zip->open($zipTempFile, ZipArchive::CREATE);
+
+        // Add default files (.env config file and this CLI)
         $zip->addFile($this->appDir . DIRECTORY_SEPARATOR . '.env', '.env');
+        $zip->addFile($this->appDir . DIRECTORY_SEPARATOR . 'scripts' . DIRECTORY_SEPARATOR . 'run', 'run');
 
         if ($handleDatabase) {
+            echo "Dumping the database via mysqldump...\n";
+            $dumpTempFile = $this->createDatabaseDump();
+            echo "Adding database dump to backup archive...\n";
             $zip->addFile($dumpTempFile, 'db.sql');
+            // Delete our temporary DB dump file
+            unlink($dumpTempFile);
         }
 
         if ($handleUploads) {
+            echo "Adding BookStack upload folders to backup archive...\n";
             $this->addUploadFoldersToZip($zip);
         }
 
+        if ($handleThemes) {
+            echo "Adding BookStack theme folders to backup archive...\n";
+            $this->addFolderToZipRecursive($zip, implode(DIRECTORY_SEPARATOR, [$this->appDir, 'themes']), 'themes');
+        }
+
         // Close off our zip and move it to the required location
         $zip->close();
         rename($zipTempFile, $zipOutFile);
 
-        // Delete our temporary DB dump file
-        unlink($dumpTempFile);
-
         // Announce end and display errors
-        echo "Finished";
+        echo "Backup finished.\nOutput ZIP saved to: {$zipOutFile}\n";
+    }
+
+    /**
+     * Ensure the required PHP extensions are installed for this command.
+     * @throws CommandError
+     */
+    protected function ensureRequiredExtensionInstalled(): void
+    {
+        if (!extension_loaded('zip')) {
+            throw new CommandError('The "zip" PHP extension is required to run this 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.
+     * @throws CommandError
      */
     protected function buildZipFilePath(string $suggestedOutPath): string
     {
@@ -72,14 +98,14 @@ final class BackupCommand
                 $zipDir = realpath(dirname($suggestedOutPath));
                 $zipName = basename($suggestedOutPath);
             } else {
-                // TODO - Handle not found output
+                throw new CommandError("Could not resolve provided [{$suggestedOutPath}] path to an existing folder.");
             }
         }
 
         $fullPath = $zipDir . DIRECTORY_SEPARATOR . $zipName;
 
         if (file_exists($fullPath)) {
-            // TODO
+            throw new CommandError("Target ZIP output location at [{$fullPath}] already exists.");
         }
 
         return $fullPath;
@@ -91,61 +117,96 @@ final class BackupCommand
      */
     protected function addUploadFoldersToZip(ZipArchive $zip): void
     {
-        $fileDirs = [
-            $this->appDir . DIRECTORY_SEPARATOR . 'public' . DIRECTORY_SEPARATOR . 'uploads' => 'public/uploads',
-            $this->appDir . DIRECTORY_SEPARATOR . 'storage' . DIRECTORY_SEPARATOR . 'uploads' => 'storage/uploads',
-        ];
+        $this->addFolderToZipRecursive($zip, implode(DIRECTORY_SEPARATOR, [$this->appDir, 'public', 'uploads']), 'public/uploads');
+        $this->addFolderToZipRecursive($zip, implode(DIRECTORY_SEPARATOR, [$this->appDir, 'storage', 'uploads']), 'storage/uploads');
+    }
 
-        foreach ($fileDirs as $fullFileDir => $relativeFileDir) {
-            $dirIter = new RecursiveDirectoryIterator($fullFileDir);
-            $fileIter = new \RecursiveIteratorIterator($dirIter);
-            /** @var SplFileInfo $file */
-            foreach ($fileIter as $file) {
-                if (!$file->isDir()) {
-                    $zip->addFile($file->getPathname(), $relativeFileDir . '/' . $fileIter->getSubPathname());
-                }
+    /**
+     * Recursively add all contents of the given dirPath to the provided zip file
+     * with a zip location of the targetZipPath.
+     */
+    protected function addFolderToZipRecursive(ZipArchive $zip, string $dirPath, string $targetZipPath): void
+    {
+        $dirIter = new RecursiveDirectoryIterator($dirPath);
+        $fileIter = new \RecursiveIteratorIterator($dirIter);
+        /** @var SplFileInfo $file */
+        foreach ($fileIter as $file) {
+            if (!$file->isDir()) {
+                $zip->addFile($file->getPathname(), $targetZipPath . '/' . $fileIter->getSubPathname());
             }
         }
     }
 
     /**
      * Create a database dump and return the path to the dumped SQL output.
+     * @throws CommandError
      */
     protected function createDatabaseDump(): string
     {
-        $dbHost = ($_SERVER['DB_HOST'] ?? '');
-        $dbUser = ($_SERVER['DB_USERNAME'] ?? '');
-        $dbPass = ($_SERVER['DB_PASSWORD'] ?? '');
-        $dbDatabase = ($_SERVER['DB_DATABASE'] ?? '');
+        $dbOptions = [
+            'host' => ($_SERVER['DB_HOST'] ?? ''),
+            'username' => ($_SERVER['DB_USERNAME'] ?? ''),
+            'password' => ($_SERVER['DB_PASSWORD'] ?? ''),
+            'database' => ($_SERVER['DB_DATABASE'] ?? ''),
+        ];
+
+        foreach ($dbOptions as $name => $option) {
+            if (!$option) {
+                throw new CommandError("Could not find a value for the database {$name}");
+            }
+        }
 
         // Create a mysqldump for the BookStack database
         $executableFinder = new ExecutableFinder();
-        $mysqldumpPath = $executableFinder->find('mysqldump');
+        $mysqldumpPath = $executableFinder->find('mysqldump', '/usr/bin/mysqldump');
+
+        if (!is_file($mysqldumpPath)) {
+            throw new CommandError('Could not locate "mysqldump" program');
+        }
 
         $process = new Process([
             $mysqldumpPath,
-            '-h', $dbHost,
-            '-u', $dbUser,
-            '-p' . $dbPass,
+            '-h', $dbOptions['host'],
+            '-u', $dbOptions['username'],
+            '-p' . $dbOptions['password'],
             '--single-transaction',
             '--no-tablespaces',
-            $dbDatabase,
+            $dbOptions['database'],
         ]);
+        $process->setTimeout(240);
+        $process->setIdleTimeout(5);
         $process->start();
 
         $errors = "";
+        $hasOutput = false;
         $dumpTempFile = tempnam(sys_get_temp_dir(), 'bsbackup');
         $dumpTempFileResource = fopen($dumpTempFile, 'w');
-        foreach ($process as $type => $data) {
-            if ($process::OUT === $type) {
-                fwrite($dumpTempFileResource, $data);
-            } else { // $process::ERR === $type
-                $errors .= $data . "\n";
+        try {
+            foreach ($process as $type => $data) {
+                if ($process::OUT === $type) {
+                    fwrite($dumpTempFileResource, $data);
+                    $hasOutput = true;
+                } else { // $process::ERR === $type
+                    $errors .= $data . "\n";
+                }
+            }
+        } catch (ProcessTimedOutException $timedOutException) {
+            fclose($dumpTempFileResource);
+            unlink($dumpTempFile);
+            if (!$hasOutput) {
+                throw new CommandError("mysqldump operation timed-out.\nNo data has been received so the connection to your database may have failed.");
+            } else {
+                throw new CommandError("mysqldump operation timed-out after data was received.");
             }
         }
+
         fclose($dumpTempFileResource);
 
-        // TODO - Throw errors if existing
+        if ($errors) {
+            unlink($dumpTempFile);
+            throw new CommandError("Failed mysqldump with errors:\n" . $errors);
+        }
+
         return $dumpTempFile;
     }
 }
diff --git a/scripts/Commands/CommandError.php b/scripts/Commands/CommandError.php
new file mode 100644
index 0000000..fdd218a
--- /dev/null
+++ b/scripts/Commands/CommandError.php
@@ -0,0 +1,5 @@
+<?php
+
+namespace Cli\Commands;
+
+class CommandError extends \Exception {}
\ No newline at end of file
diff --git a/scripts/run b/scripts/run
index ef25ce7..ff38c81 100644
--- a/scripts/run
+++ b/scripts/run
@@ -8,6 +8,7 @@ if (php_sapi_name() !== 'cli') {
 require __DIR__ . '/vendor/autoload.php';
 
 use Cli\Commands\BackupCommand;
+use Cli\Commands\CommandError;
 use Minicli\App;
 
 // Get the directory of the CLI "entrypoint", adjusted to be the real
@@ -28,4 +29,10 @@ $app->setSignature('./run');
 
 $app->registerCommand('backup', [new BackupCommand($bsDir), 'handle']);
 
-$app->runCommand($argv);
+try {
+    $app->runCommand($argv);
+} catch (CommandError $error) {
+    fwrite(STDERR, "An error occurred when attempting to run a command:\n");
+    fwrite(STDERR, $error->getMessage() . "\n");
+    exit(1);
+}