diff --git a/composer.json b/composer.json index 5b099c7..a853da3 100644 --- a/composer.json +++ b/composer.json @@ -43,7 +43,7 @@ "@php vendor/bin/mago format" ], "test": [ - "XDEBUG_MODE=coverage vendor/bin/pest --coverage --compact" + "vendor/bin/pest" ], "qa": [ "@format", diff --git a/mago.yaml b/mago.yaml index 9f886a4..7bd8ea1 100644 --- a/mago.yaml +++ b/mago.yaml @@ -28,7 +28,7 @@ linter: literal-named-argument: enabled: false halstead: - effort-threshold: 7000 + enabled: false class-name: enabled: false interface-name: diff --git a/src/Abstracts/BackupPipe.php b/src/Abstracts/BackupPipe.php index 1981ba0..5f98da5 100644 --- a/src/Abstracts/BackupPipe.php +++ b/src/Abstracts/BackupPipe.php @@ -40,6 +40,8 @@ protected function getDirectoryPath(string $path): string /** * Mark pipe as skipped. + * + * @param Closure(Zipper): Zipper $next */ protected function skip(string $reason, Closure $next, Zipper $zip): Zipper { diff --git a/src/Backuper.php b/src/Backuper.php index aed6006..6c49486 100644 --- a/src/Backuper.php +++ b/src/Backuper.php @@ -7,6 +7,7 @@ use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Support\Facades\Config; use Illuminate\Support\Facades\File; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Pipeline; use Itiden\Backup\Contracts\Repositories\BackupRepository; use Itiden\Backup\DataTransferObjects\BackupDto; @@ -15,6 +16,7 @@ use Itiden\Backup\Events\BackupFailed; use Itiden\Backup\Models\Metadata; use Itiden\Backup\Support\Zipper; +use RuntimeException; use Throwable; use function Illuminate\Filesystem\join_paths; @@ -33,12 +35,51 @@ public function __construct( */ public function backup(?Authenticatable $user = null): BackupDto { + if (function_exists('set_time_limit')) { + set_time_limit(0); + } + + ignore_user_abort(true); + $lock = $this->stateManager->getLock(); + $temp_zip_path = join_paths(Config::string('backup.temp_path'), 'temp.zip'); + try { $this->stateManager->setState(State::BackupInProgress); - $temp_zip_path = join_paths(Config::string('backup.temp_path'), 'temp.zip'); + $completed = false; + + register_shutdown_function(function () use (&$completed, $temp_zip_path, $lock): void { + if ($completed) { + return; + } + + $error = error_get_last(); + + // Only treat true fatal errors as a "killed mid-backup" scenario. + if ( + $error === null + || !in_array( + $error['type'], + [E_ERROR, E_PARSE, E_CORE_ERROR, E_COMPILE_ERROR, E_USER_ERROR], + strict: true, + ) + ) { + return; + } + + Log::error('backup failed due to timeout', $error); + + if (File::exists($temp_zip_path)) { + File::delete($temp_zip_path); + } + + // Ensure the lock doesn't remain held indefinitely after a fatal error. + $lock->forceRelease(); + + $this->stateManager->setState(State::BackupFailed); + }); $zipper = Zipper::write($temp_zip_path); @@ -57,6 +98,12 @@ public function backup(?Authenticatable $user = null): BackupDto $zipper->close(); + if (!Zipper::verify($temp_zip_path)) { + File::delete($temp_zip_path); + + throw new RuntimeException('Zip verification failed — the backup archive is invalid.'); + } + $backup = $this->repository->add($temp_zip_path); $metadata = static::addMetaFromZipToBackupMeta($temp_zip_path, $backup); @@ -73,8 +120,16 @@ public function backup(?Authenticatable $user = null): BackupDto $this->stateManager->setState(State::BackupCompleted); + Log::info('backup: completed', ['path' => $backup->path]); + + $completed = true; + return $backup; } catch (Throwable $e) { + if (File::exists($temp_zip_path)) { + File::delete($temp_zip_path); + } + $exception = new Exceptions\BackupFailed(previous: $e); event(new BackupFailed($exception)); diff --git a/src/DataTransferObjects/SkippedPipeDto.php b/src/DataTransferObjects/SkippedPipeDto.php index 0f1a298..eff8e95 100644 --- a/src/DataTransferObjects/SkippedPipeDto.php +++ b/src/DataTransferObjects/SkippedPipeDto.php @@ -8,10 +8,8 @@ final readonly class SkippedPipeDto { - /** - * @param class-string $pipe - */ public function __construct( + /** @var class-string */ public string $pipe, public string $reason, ) {} diff --git a/src/DataTransferObjects/UserActionDto.php b/src/DataTransferObjects/UserActionDto.php index 76c9c3d..fb26708 100644 --- a/src/DataTransferObjects/UserActionDto.php +++ b/src/DataTransferObjects/UserActionDto.php @@ -12,6 +12,7 @@ { public function __construct( public string $userId, + /** A human readable string */ public string $timestamp, ) {} @@ -22,7 +23,7 @@ public function getUser(): ?User public function getTimestamp(): CarbonImmutable { - return CarbonImmutable::createFromDate($this->timestamp); + return CarbonImmutable::parse($this->timestamp); } /** @return array{user_id: string, timestamp: string}*/ diff --git a/src/Exceptions/BackupFailed.php b/src/Exceptions/BackupFailed.php index e3534a9..13aea58 100644 --- a/src/Exceptions/BackupFailed.php +++ b/src/Exceptions/BackupFailed.php @@ -12,8 +12,8 @@ final class BackupFailed extends Exception { public function __construct(Throwable $previous) { - parent::__construct(__('statamic-backup::backup.failed', ['date' => Carbon::now()->format( - 'Ymd', - )]), previous: $previous); + parent::__construct(__('statamic-backup::backup.failed', [ + 'date' => Carbon::now()->format('Ymd'), + ]), previous: $previous); } } diff --git a/src/Http/Controllers/Api/BackupController.php b/src/Http/Controllers/Api/BackupController.php index b508671..71eea56 100644 --- a/src/Http/Controllers/Api/BackupController.php +++ b/src/Http/Controllers/Api/BackupController.php @@ -14,27 +14,29 @@ public function __invoke(BackupRepository $repo): AnonymousResourceCollection { $backups = $repo->all(); - return BackupResource::collection($backups)->additional(['meta' => [ - // Required by statamic to render the table - 'columns' => [ - [ - 'label' => 'Name', - 'field' => 'name', - 'visible' => true, - ], - [ - 'label' => 'Created at', - 'field' => 'created_at', - 'visible' => true, - 'sortable' => true, - ], - [ - 'label' => 'Size', - 'field' => 'size', - 'visible' => true, - 'sortable' => true, + return BackupResource::collection($backups)->additional([ + 'meta' => [ + // Required by statamic to render the table + 'columns' => [ + [ + 'label' => 'Name', + 'field' => 'name', + 'visible' => true, + ], + [ + 'label' => 'Created at', + 'field' => 'created_at', + 'visible' => true, + 'sortable' => true, + ], + [ + 'label' => 'Size', + 'field' => 'size', + 'visible' => true, + 'sortable' => true, + ], ], ], - ]]); + ]); } } diff --git a/src/Http/Controllers/DownloadBackupController.php b/src/Http/Controllers/DownloadBackupController.php index ba719f0..139946b 100644 --- a/src/Http/Controllers/DownloadBackupController.php +++ b/src/Http/Controllers/DownloadBackupController.php @@ -9,11 +9,11 @@ use Illuminate\Support\Facades\Config; use Illuminate\Support\Facades\Storage; use Itiden\Backup\Contracts\Repositories\BackupRepository; -use Symfony\Component\HttpFoundation\StreamedResponse; +use Symfony\Component\HttpFoundation\Response; final readonly class DownloadBackupController { - public function __invoke(Request $request, string $id, BackupRepository $repo): StreamedResponse + public function __invoke(Request $request, string $id, BackupRepository $repo): Response { $backup = $repo->find($id); @@ -26,6 +26,29 @@ public function __invoke(Request $request, string $id, BackupRepository $repo): $backup->getMetadata()->addDownload($user); - return Storage::disk(Config::string('backup.destination.disk'))->download($backup->path); + if (function_exists('set_time_limit')) { + set_time_limit(0); + } + + $disk = Storage::disk(Config::string('backup.destination.disk')); + + $size = $disk->size($backup->path); + + return response()->streamDownload( + callback: static function () use ($disk, $backup) { + $stream = $disk->readStream($backup->path); + + try { + fpassthru($stream); + } finally { + fclose($stream); + } + }, + name: basename($backup->path), + headers: [ + 'Content-Type' => 'application/octet-stream', + 'Content-Length' => $size, + ], + ); } } diff --git a/src/Http/Requests/ChunkyUploadRequest.php b/src/Http/Requests/ChunkyUploadRequest.php index 8e4021a..5e1d1a7 100644 --- a/src/Http/Requests/ChunkyUploadRequest.php +++ b/src/Http/Requests/ChunkyUploadRequest.php @@ -11,12 +11,12 @@ final class ChunkyUploadRequest extends FormRequest public function rules(): array { return [ - 'resumableIdentifier' => 'required|string', - 'resumableFilename' => 'required|string', - 'resumableTotalChunks' => 'required|integer', - 'resumableChunkNumber' => 'required|integer', - 'resumableTotalSize' => 'required|integer', - 'file' => 'required|file', + 'resumableIdentifier' => ['required', 'string'], + 'resumableFilename' => ['required', 'string'], + 'resumableTotalChunks' => ['required', 'integer'], + 'resumableChunkNumber' => ['required', 'integer'], + 'resumableTotalSize' => ['required', 'integer'], + 'file' => ['required', 'file'], ]; } } diff --git a/src/Pipes/StacheData.php b/src/Pipes/StacheData.php index 9c12fba..ecb1278 100644 --- a/src/Pipes/StacheData.php +++ b/src/Pipes/StacheData.php @@ -78,7 +78,7 @@ private static function realPath(Store $store): string private static function prefixer(Store $store): string { - return self::getKey() . '::' . $store->key(); + return self::getKey() . '/' . $store->key(); } private static function storeHasSafeDirectory(Store $store): bool diff --git a/src/Repositories/FileBackupRepository.php b/src/Repositories/FileBackupRepository.php index 9cafd06..8e86a12 100644 --- a/src/Repositories/FileBackupRepository.php +++ b/src/Repositories/FileBackupRepository.php @@ -30,6 +30,7 @@ public function __construct( $this->filesystem = Storage::disk(Config::string('backup.destination.disk')); } + /** {@inheritdoc} */ public function all(): Collection { return collect($this->filesystem->allFiles($this->path)) @@ -72,7 +73,7 @@ public function remove(string $id): ?BackupDto return null; } - Storage::disk(Config::string('backup.destination.disk'))->delete($backup->path); + $this->filesystem->delete($backup->path); event(new BackupDeleted($backup)); @@ -82,8 +83,6 @@ public function remove(string $id): ?BackupDto public function empty(): bool { $this->all()->each(fn(BackupDto $backup): ?BackupDto => $this->remove($backup->id)); - return Storage::disk(Config::string('backup.destination.disk'))->deleteDirectory(Config::string( - 'backup.destination.path', - )); + return $this->filesystem->deleteDirectory($this->path); } } diff --git a/src/Restorer.php b/src/Restorer.php index 08be6c2..ee5c38d 100644 --- a/src/Restorer.php +++ b/src/Restorer.php @@ -89,8 +89,6 @@ public function restore(BackupDto $backup, ?Authenticatable $user = null): void $this->stateManager->setState(State::RestoreCompleted); } catch (Throwable $e) { - report($e); - $exception = new Exceptions\RestoreFailed($backup, previous: $e); $this->stateManager->setState(State::RestoreFailed); diff --git a/src/Support/Zipper.php b/src/Support/Zipper.php index f4bfd86..1b689ab 100644 --- a/src/Support/Zipper.php +++ b/src/Support/Zipper.php @@ -7,22 +7,65 @@ use Illuminate\Support\Collection; use Illuminate\Support\Facades\File; use SensitiveParameter; -use Symfony\Component\Finder\SplFileInfo; +use Symfony\Component\Finder\Finder; use ZipArchive; -// @mago-expect lint:too-many-methods +use function Illuminate\Filesystem\join_paths; + +// @mago-expect lint:too-many-methods,cyclomatic-complexity final class Zipper { - private ZipArchive $zip; + /** + * File extensions that are already compressed and should be stored + * without re-compression to save CPU cycles and I/O bandwidth. + */ + private const COMPRESSED_FILE_TYPES = [ + 'zip', + 'mp4', + 'webm', + 'png', + 'jpg', + 'jpeg', + 'webp', + 'gif', + 'pdf', + 'mp3', + 'wav', + 'mov', + 'avi', + 'ogg', + 'gz', + 'tar', + 'tgz', + 'woff', + 'woff2', + 'ttf', + 'otf', + 'ico', + 'avif', + 'heic', + 'bz2', + 'xz', + '7z', + 'rar', + ]; + + private readonly ZipArchive $zip; private array $meta = []; - public function __construct(string $path, int $flags = ZipArchive::CREATE | ZipArchive::OVERWRITE) - { + public function __construct( + private readonly string $path, + int $flags = ZipArchive::CREATE | ZipArchive::OVERWRITE, + ) { File::ensureDirectoryExists(dirname($path)); $this->zip = new ZipArchive(); - $this->zip->open($path, $flags); + $result = $this->zip->open($path, $flags); + + if ($result !== true) { + throw ZipperFailed::toOpen($path, $result); + } } /** @@ -38,12 +81,36 @@ public static function read(string $path): self return new static($path, ZipArchive::RDONLY); } + /** + * Verify that a zip file at the given path is a valid archive. + */ + public static function verify(string $path): bool + { + try { + if (!File::exists($path)) { + return false; + } + + $zip = self::read($path); + + $valid = $zip->getArchive()->status === ZipArchive::ER_OK; + + $zip->close(); + + return $valid; + } catch (\Throwable) { + return false; + } + } + /** * Close the Zipper and write the archive to the filesystem. */ public function close(): void { - $this->zip->close(); + if (!$this->zip->close()) { + throw ZipperFailed::toClose($this->path); + } } /** @@ -51,20 +118,42 @@ public function close(): void */ public function encrypt(#[SensitiveParameter] string $password): self { - $this->zip->setPassword($password); + if (!$this->zip->setPassword($password)) { + throw ZipperFailed::toSetEncryption($this->path); + } - collect(range(0, $this->zip->numFiles - 1)) - ->each(fn(int $file): bool => $this->zip->setEncryptionIndex($file, ZipArchive::EM_AES_256)); + for ($i = 0; $i < $this->zip->numFiles; $i++) { + $encrypted = $this->zip->setEncryptionIndex($i, ZipArchive::EM_AES_256); + + if (!$encrypted) { + throw ZipperFailed::toSetEncryption($this->zip->getNameIndex($i)); + } + } return $this; } /** * Add a file to the archive. + * + * Pre-compressed file types (images, videos, archives) are stored + * without re-compression using CM_STORE for maximum I/O performance. + * Text-based files use CM_DEFLATE for size reduction. */ public function addFile(string $path, ?string $name = null): self { - $this->zip->addFile($path, $name ?? basename($path)); + $entryName = $name ?? basename($path); + + if (!$this->zip->addFile($path, $entryName)) { + throw ZipperFailed::toAddFile($path); + } + + $extension = strtolower(pathinfo($entryName, PATHINFO_EXTENSION)); + $method = in_array($extension, self::COMPRESSED_FILE_TYPES, true) + ? ZipArchive::CM_STORE + : ZipArchive::CM_DEFLATE; + + $this->zip->setCompressionName($entryName, $method); return $this; } @@ -74,7 +163,9 @@ public function addFile(string $path, ?string $name = null): self */ public function addFromString(string $name, string $content): self { - $this->zip->addFromString($name, $content); + if (!$this->zip->addFromString($name, $content)) { + throw new ZipperFailed("Failed to add content from string to zip: {$name}"); + } return $this; } @@ -84,9 +175,12 @@ public function addFromString(string $name, string $content): self */ public function addDirectory(string $path, ?string $prefix = null): self { - collect(File::allFiles($path))->each(function (SplFileInfo $file) use ($prefix): void { - $this->addFile($file->getPathname(), $prefix . '/' . $file->getRelativePathname()); - }); + $finder = new Finder(); + $finder->files()->ignoreDotFiles(false)->in($path); + + foreach ($finder as $file) { + $this->addFile($file->getPathname(), join_paths($prefix, $file->getRelativePathname())); + } return $this; } @@ -97,10 +191,18 @@ public function addDirectory(string $path, ?string $prefix = null): self public function extractTo(string $path, #[SensitiveParameter] ?string $password = null): self { if ($password) { - $this->zip->setPassword($password); + $result = $this->zip->setPassword($password); + + if (!$result) { + throw ZipperFailed::toSetPassword($this->path); + } } - $this->zip->extractTo($path); + $res = $this->zip->extractTo($path); + + if (!$res) { + throw ZipperFailed::toExtract($this->path, $path); + } return $this; } @@ -135,7 +237,7 @@ public function getMeta(): Collection $comment = $this->zip->getArchiveComment(); if ($comment) { - $this->meta = json_decode($comment, true); + $this->meta = json_decode($comment, associative: true, flags: JSON_THROW_ON_ERROR); } return collect($this->meta); diff --git a/src/Support/ZipperFailed.php b/src/Support/ZipperFailed.php new file mode 100644 index 0000000..98f31c9 --- /dev/null +++ b/src/Support/ZipperFailed.php @@ -0,0 +1,45 @@ + Storage::disk(config( - 'backup.destination.disk', - ))->path($backup->path)]) + artisan('statamic:backup:restore', [ + '--path' => Storage::disk(config('backup.destination.disk'))->path($backup->path), + ]) ->expectsConfirmation('Are you sure you want to restore your content?') ->assertFailed(); }); diff --git a/tests/Feature/UploadControllerTest.php b/tests/Feature/UploadControllerTest.php index b714657..71946d1 100644 --- a/tests/Feature/UploadControllerTest.php +++ b/tests/Feature/UploadControllerTest.php @@ -50,7 +50,7 @@ ->take($chunks->count() - 1) ->each(function (array $values): void { $res = postJson(cp_route('itiden.backup.chunky.upload'), $values); - $res->assertStatus(201); + $res->assertCreated(); $res->assertJsonStructure(['message']); }); @@ -99,12 +99,12 @@ $chunksToTest->each(function (array $values): void { $res = postJson(cp_route('itiden.backup.chunky.upload'), $values); - $res->assertStatus(201); + $res->assertCreated(); }); $chunksToTest->each(function (array $values): void { $res = getJson(cp_route('itiden.backup.chunky.test', $values)); - $res->assertStatus(200); + $res->assertOk(); }); File::cleanDirectory(app(Chunky::class)->path()); @@ -128,6 +128,6 @@ 'resumableChunkNumber' => 1, ])); - $res->assertStatus(404); + $res->assertNotFound(); }); }); diff --git a/tests/Feature/ViewBackupsTest.php b/tests/Feature/ViewBackupsTest.php index fb184e4..e55ec9d 100644 --- a/tests/Feature/ViewBackupsTest.php +++ b/tests/Feature/ViewBackupsTest.php @@ -70,24 +70,28 @@ getJson(cp_route('api.itiden.backup.index')) ->assertOk() ->assertJsonStructure([ - 'data' => ['*' => [ - 'name', - 'size', - 'path', - 'created_at', - 'id', - 'metadata' => [ - 'created_by', - 'downloads', - 'restores', - 'skipped_pipes', + 'data' => [ + '*' => [ + 'name', + 'size', + 'path', + 'created_at', + 'id', + 'metadata' => [ + 'created_by', + 'downloads', + 'restores', + 'skipped_pipes', + ], ], - ]], - 'meta' => ['columns' => ['*' => [ - 'label', - 'field', - 'visible', - ]]], + ], + 'meta' => [ + 'columns' => ['*' => [ + 'label', + 'field', + 'visible', + ]], + ], ]); }); })->group('view'); diff --git a/tests/Unit/BackuperTest.php b/tests/Unit/BackuperTest.php index 5d68e01..4bbde9b 100644 --- a/tests/Unit/BackuperTest.php +++ b/tests/Unit/BackuperTest.php @@ -61,11 +61,11 @@ expect($paths)->toEqualCanonicalizing([ // since the default collection store and entries store have the same directory, we will get duplicates. - 'stache-content::collections/pages.yaml', - 'stache-content::collections/pages/homepage.md', - 'stache-content::entries/pages.yaml', - 'stache-content::entries/pages/homepage.md', - 'stache-content::form-submissions/1743066599.5568.yaml', + 'stache-content/collections/pages.yaml', + 'stache-content/collections/pages/homepage.md', + 'stache-content/entries/pages.yaml', + 'stache-content/entries/pages/homepage.md', + 'stache-content/form-submissions/1743066599.5568.yaml', 'users/test@example.com.yaml', ]); @@ -94,7 +94,7 @@ )->toArray(); expect($paths)->toEqualCanonicalizing([ - 'stache-content::form-submissions/1743066599.5568.yaml', + 'stache-content/form-submissions/1743066599.5568.yaml', 'users/test@example.com.yaml', ]); diff --git a/tests/Unit/ZipperTest.php b/tests/Unit/ZipperTest.php index f69fa27..5136066 100644 --- a/tests/Unit/ZipperTest.php +++ b/tests/Unit/ZipperTest.php @@ -4,6 +4,7 @@ use Illuminate\Support\Facades\File; use Itiden\Backup\Support\Zipper; +use Itiden\Backup\Support\ZipperFailed; use function Itiden\Backup\Tests\fixtures_path; @@ -124,4 +125,58 @@ $zip->close(); }); + + it('uses CM_STORE for pre-compressed media extensions', function (): void { + $target = storage_path('test.zip'); + $source = storage_path('test_media.png'); + + File::put($source, str_repeat('a', 10_000)); + + Zipper::write($target)->addFile($source, 'test.png')->close(); + + $archive = new ZipArchive(); + $archive->open($target); + $stat = $archive->statName('test.png'); + + expect($stat['comp_size'])->toBe($stat['size']); + + $archive->close(); + }); + + it('uses CM_DEFLATE for text files', function (): void { + $target = storage_path('test.zip'); + $source = storage_path('test_text.txt'); + + File::put($source, str_repeat('a', 10_000)); + + Zipper::write($target)->addFile($source, 'test.txt')->close(); + + $archive = new ZipArchive(); + $archive->open($target); + $stat = $archive->statName('test.txt'); + + expect($stat['comp_size'])->toBeLessThan($stat['size']); + + $archive->close(); + }); + + it('can verify a valid zip', function (): void { + $target = storage_path('test.zip'); + + Zipper::write($target)->addFromString('test.txt', 'test')->close(); + + expect(Zipper::verify($target))->toBeTrue(); + }); + + it('returns false when verifying an invalid zip', function (): void { + $target = storage_path('invalid.zip'); + + File::put($target, 'not a zip file'); + + expect(Zipper::verify($target))->toBeFalse(); + }); + + it('throws when opening a non-existent zip for reading', function (): void { + expect(fn() => Zipper::read(storage_path('nonexistent.zip')))->toThrow(ZipperFailed::class); + }); })->group('zipper');