diff --git a/app/Extensions/Filesystem/S3Filesystem.php b/app/Extensions/Filesystem/S3Filesystem.php index 64aa95bb48..daf037e9fa 100644 --- a/app/Extensions/Filesystem/S3Filesystem.php +++ b/app/Extensions/Filesystem/S3Filesystem.php @@ -2,11 +2,18 @@ namespace App\Extensions\Filesystem; +use Aws\CommandInterface; +use Aws\Result; use Aws\S3\S3ClientInterface; +use GuzzleHttp\Client as GuzzleClient; use League\Flysystem\AwsS3V3\AwsS3V3Adapter; +use RuntimeException; +use SimpleXMLElement; class S3Filesystem extends AwsS3V3Adapter { + private ?GuzzleClient $guzzle = null; + /** * @param array $options */ @@ -26,6 +33,18 @@ public function __construct( ); } + private function getGuzzleClient(): GuzzleClient + { + if ($this->guzzle === null) { + $this->guzzle = new GuzzleClient([ + 'timeout' => 30, + 'connect_timeout' => 10, + ]); + } + + return $this->guzzle; + } + public function getClient(): S3ClientInterface { return $this->client; @@ -35,4 +54,78 @@ public function getBucket(): string { return $this->bucket; } + + /** + * Execute an S3 command using a presigned URL for maximum compatibility + * with S3-compatible providers. + * + * @return Result> + */ + public function executeS3Command(CommandInterface $command): Result + { + $presignedRequest = $this->client->createPresignedRequest($command, '+60 minutes'); + + $response = $this->getGuzzleClient()->send($presignedRequest); + + $body = (string) $response->getBody(); + $commandName = $command->getName(); + + // S3's CompleteMultipartUpload can return HTTP 200 with an body + if ($body !== '' && str_contains($body, '')) { + throw new RuntimeException("S3 returned an error for $commandName: $body"); + } + + return new Result($this->parseS3Response($commandName, $body)); + } + + /** + * Parse the XML response body based on the S3 command type. + * + * @return array + */ + private function parseS3Response(string $commandName, string $body): array + { + if ($body === '') { + return []; + } + + $xml = @simplexml_load_string($body); + if ($xml === false) { + throw new RuntimeException("Failed to parse S3 XML response for $commandName: $body"); + } + + return match ($commandName) { + 'CreateMultipartUpload' => $this->parseCreateMultipartUpload($xml), + 'ListParts' => $this->parseListParts($xml), + 'CompleteMultipartUpload' => [], + default => [], + }; + } + + /** + * @return array{UploadId: string} + */ + private function parseCreateMultipartUpload(SimpleXMLElement $xml): array + { + return [ + 'UploadId' => (string) $xml->UploadId, + ]; + } + + /** + * @return array{Parts: array} + */ + private function parseListParts(SimpleXMLElement $xml): array + { + $parts = []; + + foreach ($xml->Part as $part) { + $parts[] = [ + 'ETag' => (string) $part->ETag, + 'PartNumber' => (int) $part->PartNumber, + ]; + } + + return ['Parts' => $parts]; + } } diff --git a/app/Http/Controllers/Api/Remote/Backups/BackupRemoteUploadController.php b/app/Http/Controllers/Api/Remote/Backups/BackupRemoteUploadController.php index fa149c31e6..ad23875a59 100644 --- a/app/Http/Controllers/Api/Remote/Backups/BackupRemoteUploadController.php +++ b/app/Http/Controllers/Api/Remote/Backups/BackupRemoteUploadController.php @@ -91,7 +91,7 @@ public function __invoke(Request $request, string $backup): JsonResponse } // Execute the CreateMultipartUpload request - $result = $client->execute($client->getCommand('CreateMultipartUpload', $params)); + $result = $adapter->executeS3Command($client->getCommand('CreateMultipartUpload', $params)); // Get the UploadId from the CreateMultipartUpload request, this is needed to create // the other presigned urls. diff --git a/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php b/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php index ba69fd0047..9c2033fe89 100644 --- a/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php +++ b/app/Http/Controllers/Api/Remote/Backups/BackupStatusController.php @@ -138,7 +138,7 @@ protected function completeMultipartUpload(Backup $backup, S3Filesystem $adapter $client = $adapter->getClient(); if (!$successful) { - $client->execute($client->getCommand('AbortMultipartUpload', $params)); + $adapter->executeS3Command($client->getCommand('AbortMultipartUpload', $params)); return; } @@ -149,7 +149,7 @@ protected function completeMultipartUpload(Backup $backup, S3Filesystem $adapter ]; if (is_null($parts)) { - $params['MultipartUpload']['Parts'] = $client->execute($client->getCommand('ListParts', $params))['Parts']; + $params['MultipartUpload']['Parts'] = $adapter->executeS3Command($client->getCommand('ListParts', $params))['Parts']; } else { foreach ($parts as $part) { $params['MultipartUpload']['Parts'][] = [ @@ -159,6 +159,6 @@ protected function completeMultipartUpload(Backup $backup, S3Filesystem $adapter } } - $client->execute($client->getCommand('CompleteMultipartUpload', $params)); + $adapter->executeS3Command($client->getCommand('CompleteMultipartUpload', $params)); } } diff --git a/app/Services/Activity/ActivityLogService.php b/app/Services/Activity/ActivityLogService.php index 9cf8e2d1ef..2a06427fc4 100644 --- a/app/Services/Activity/ActivityLogService.php +++ b/app/Services/Activity/ActivityLogService.php @@ -10,7 +10,6 @@ use Illuminate\Contracts\Auth\Factory as AuthFactory; use Illuminate\Database\ConnectionInterface; use Illuminate\Database\Eloquent\Model; -use Illuminate\Support\Arr; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Request; use Throwable; @@ -71,7 +70,7 @@ public function description(?string $description): self */ public function subject(...$subjects): self { - foreach (Arr::wrap($subjects) as $subject) { + foreach ($subjects as $subject) { if (is_null($subject)) { continue; } diff --git a/app/Services/Backups/DeleteBackupService.php b/app/Services/Backups/DeleteBackupService.php index c5064a2d09..cf62f1f8cf 100644 --- a/app/Services/Backups/DeleteBackupService.php +++ b/app/Services/Backups/DeleteBackupService.php @@ -7,7 +7,6 @@ use App\Extensions\Filesystem\S3Filesystem; use App\Models\Backup; use App\Repositories\Daemon\DaemonBackupRepository; -use Aws\S3\S3Client; use Exception; use Illuminate\Database\ConnectionInterface; use Illuminate\Http\Response; @@ -72,14 +71,12 @@ protected function deleteFromS3(Backup $backup): void /** @var S3Filesystem $adapter */ $adapter = $this->manager->adapter(Backup::ADAPTER_AWS_S3); - - /** @var S3Client $client */ $client = $adapter->getClient(); - $client->deleteObject([ + $adapter->executeS3Command($client->getCommand('DeleteObject', [ 'Bucket' => $adapter->getBucket(), 'Key' => sprintf('%s/%s.tar.gz', $backup->server->uuid, $backup->uuid), - ]); + ])); }); } } diff --git a/tests/Integration/Services/Backups/DeleteBackupServiceTest.php b/tests/Integration/Services/Backups/DeleteBackupServiceTest.php index b8a796c2c3..55d6a6ccad 100644 --- a/tests/Integration/Services/Backups/DeleteBackupServiceTest.php +++ b/tests/Integration/Services/Backups/DeleteBackupServiceTest.php @@ -9,8 +9,10 @@ use App\Repositories\Daemon\DaemonBackupRepository; use App\Services\Backups\DeleteBackupService; use App\Tests\Integration\IntegrationTestCase; +use Aws\CommandInterface; use GuzzleHttp\Psr7\Response; use Illuminate\Http\Client\ConnectionException; +use Mockery; class DeleteBackupServiceTest extends IntegrationTestCase { @@ -92,10 +94,12 @@ public function test_s3_object_can_be_deleted(): void $manager->expects('adapter')->with(Backup::ADAPTER_AWS_S3)->andReturn($adapter); $adapter->expects('getBucket')->andReturn('foobar'); - $adapter->expects('getClient->deleteObject')->with([ + $mockCommand = Mockery::mock(CommandInterface::class); + $adapter->expects('getClient->getCommand')->with('DeleteObject', [ 'Bucket' => 'foobar', 'Key' => sprintf('%s/%s.tar.gz', $server->uuid, $backup->uuid), - ]); + ])->andReturn($mockCommand); + $adapter->expects('executeS3Command')->with($mockCommand); $this->app->make(DeleteBackupService::class)->handle($backup);