diff --git a/app/Http/Controllers/App/AssetController.php b/app/Http/Controllers/App/AssetController.php index c5c2b08a..62e2ca17 100644 --- a/app/Http/Controllers/App/AssetController.php +++ b/app/Http/Controllers/App/AssetController.php @@ -9,6 +9,7 @@ use App\Http\Requests\App\Asset\StoreChunkedAssetRequest; use App\Http\Resources\App\MediaResource; use App\Models\Media; +use App\Models\Workspace; use App\Services\Brand\SafeHttpFetcher; use App\Services\Media\ChunkedCloudUploader; use App\Services\UnsplashService; @@ -22,6 +23,7 @@ use Inertia\Response; use RuntimeException; use Symfony\Component\HttpFoundation\Response as SymfonyResponse; +use Throwable; class AssetController extends Controller { @@ -80,11 +82,12 @@ public function storeChunked(StoreChunkedAssetRequest $request, ChunkedCloudUplo $totalSize = (int) $request->validated('total_size'); $fileName = (string) $request->validated('file_name'); $chunk = $request->getContent(); - $identifier = md5($request->user()->id.$fileName.$totalSize); if ($cloudUploader->shouldUseMultipart($fileName)) { - $result = $cloudUploader->receiveChunk( + return $this->storeChunkedViaMultipart( + $cloudUploader, + $workspace, $identifier, $fileName, $chunk, @@ -92,58 +95,99 @@ public function storeChunked(StoreChunkedAssetRequest $request, ChunkedCloudUplo $rangeEnd, $totalSize, ); + } - if (! data_get($result, 'done')) { - return response()->json([ - 'done' => false, - 'progress' => data_get($result, 'progress'), - ]); - } + return $this->storeChunkedViaLocalAssemble( + $workspace, + $identifier, + $fileName, + $chunk, + $rangeStart, + $rangeEnd, + $totalSize, + ); + } + private function storeChunkedViaMultipart( + ChunkedCloudUploader $cloudUploader, + Workspace $workspace, + string $identifier, + string $fileName, + string $chunk, + int $rangeStart, + int $rangeEnd, + int $totalSize, + ): JsonResponse { + $result = $cloudUploader->receiveChunk( + $identifier, + $fileName, + $chunk, + $rangeStart, + $rangeEnd, + $totalSize, + ); + + if (! data_get($result, 'done')) { + return response()->json([ + 'done' => false, + 'progress' => data_get($result, 'progress'), + ]); + } + + $path = (string) data_get($result, 'path'); + + try { $media = $workspace->addMediaFromStoredPath( - (string) data_get($result, 'path'), + $path, $fileName, (string) data_get($result, 'mime_type'), (int) data_get($result, 'size'), 'assets', ); + } catch (Throwable $exception) { + Storage::delete($path); - return response()->json([ - 'done' => true, - 'id' => $media->id, - 'path' => $media->path, - 'url' => $media->url, - 'type' => $media->type->value, - 'mime_type' => $media->mime_type, - 'original_filename' => $media->original_filename, - 'size' => $media->size, - 'meta' => $media->meta, - 'created_at' => $media->created_at->toISOString(), - ]); + throw $exception; } + return $this->chunkedMediaResponse($media); + } + + private function storeChunkedViaLocalAssemble( + Workspace $workspace, + string $identifier, + string $fileName, + string $chunk, + int $rangeStart, + int $rangeEnd, + int $totalSize, + ): JsonResponse { $tempFile = storage_path("app/private/chunks/{$identifier}"); - $directory = dirname($tempFile); - if (! is_dir($directory)) { - mkdir($directory, 0755, true); + if (! is_dir(dirname($tempFile))) { + mkdir(dirname($tempFile), 0755, true); } file_put_contents($tempFile, $chunk, $rangeStart === 0 ? 0 : FILE_APPEND); - $isLastChunk = ($rangeEnd + 1) >= $totalSize; - - if (! $isLastChunk) { + if (($rangeEnd + 1) < $totalSize) { return response()->json([ 'done' => false, 'progress' => (int) round(($rangeEnd + 1) / $totalSize * 100), ]); } - $media = $workspace->addMediaFromPath($tempFile, $fileName, 'assets'); + try { + $media = $workspace->addMediaFromPath($tempFile, $fileName, 'assets'); + } finally { + @unlink($tempFile); + } - @unlink($tempFile); + return $this->chunkedMediaResponse($media); + } + private function chunkedMediaResponse(Media $media): JsonResponse + { return response()->json([ 'done' => true, 'id' => $media->id, diff --git a/app/Services/Media/ChunkedCloudUploader.php b/app/Services/Media/ChunkedCloudUploader.php index 088f35e5..f3f940f9 100644 --- a/app/Services/Media/ChunkedCloudUploader.php +++ b/app/Services/Media/ChunkedCloudUploader.php @@ -11,19 +11,19 @@ use Illuminate\Filesystem\AwsS3V3Adapter; use Illuminate\Support\Facades\Storage; use Illuminate\Support\Str; +use InvalidArgumentException; use RuntimeException; use Throwable; -/** - * Fast path for large uploads on object-storage disks (S3 / R2 / Spaces). - * Local and public disks keep the normal assemble-then-store flow — both work. - */ class ChunkedCloudUploader { private const CACHE_PREFIX = 'chunked-cloud-upload:'; private const CACHE_TTL_HOURS = 6; + /** S3/R2 require every non-final multipart part to be at least 5 MiB. */ + public const MIN_PART_BYTES = 5 * 1024 * 1024; + public function __construct( private readonly CacheRepository $cache, private readonly ?S3Client $client = null, @@ -32,9 +32,9 @@ public function __construct( ) {} /** - * Use S3 multipart when the default disk is object storage and the file is - * a video/PDF. Images still assemble locally (format normalization). Local - * and public disks always return false and use the regular chunk path. + * Multipart is only needed when the disk is remote object storage and the + * file will not be rewritten locally (videos/PDFs). Local, public, and + * image uploads keep the assemble-then-store path. */ public function shouldUseMultipart(string $fileName, ?string $disk = null): bool { @@ -66,24 +66,50 @@ public function receiveChunk( int $totalSize, ): array { $cacheKey = self::CACHE_PREFIX.$identifier; - $client = $this->s3(); - $bucket = $this->bucket(); + $chunkSize = strlen($chunk); + $isLastChunk = ($rangeEnd + 1) >= $totalSize; + + if ($rangeEnd < $rangeStart || $chunkSize !== ($rangeEnd - $rangeStart + 1)) { + throw new InvalidArgumentException('Chunk bytes do not match Content-Range.'); + } + + if (! $isLastChunk && $chunkSize < self::MIN_PART_BYTES) { + throw new InvalidArgumentException( + 'Non-final multipart parts must be at least '.self::MIN_PART_BYTES.' bytes.' + ); + } + + $state = $this->cache->get($cacheKey); if ($rangeStart === 0) { - $this->abortIfPresent($cacheKey); - $state = $this->startUpload($client, $bucket, $fileName, $chunk); - } else { - $state = $this->cache->get($cacheKey); + $alreadyAcceptedFirstPart = is_array($state) && (int) data_get($state, 'next_offset', 0) > 0; - if (! is_array($state)) { - throw new RuntimeException('Chunked cloud upload session expired or missing.'); + if (! $alreadyAcceptedFirstPart) { + $this->abortIfPresent($cacheKey); + $state = $this->startUpload($fileName, $chunk); + $this->cache->put($cacheKey, $state, now()->addHours(self::CACHE_TTL_HOURS)); } + } elseif (! is_array($state)) { + throw new RuntimeException('Chunked cloud upload session expired or missing.'); + } + + $nextOffset = (int) data_get($state, 'next_offset', 0); + + // Idempotent replay: client retried a chunk the server already accepted. + if ($rangeStart < $nextOffset) { + return $this->status($state, $totalSize, completed: $nextOffset >= $totalSize); + } + + if ($rangeStart !== $nextOffset) { + throw new InvalidArgumentException( + "Unexpected chunk offset {$rangeStart}, expected {$nextOffset}." + ); } $partNumber = count(data_get($state, 'parts', [])) + 1; - $result = $client->uploadPart([ - 'Bucket' => $bucket, + $result = $this->s3()->uploadPart([ + 'Bucket' => $this->bucket(), 'Key' => data_get($state, 'key'), 'UploadId' => data_get($state, 'upload_id'), 'PartNumber' => $partNumber, @@ -94,20 +120,17 @@ public function receiveChunk( 'ETag' => data_get($result, 'ETag'), 'PartNumber' => $partNumber, ]; - - $isLastChunk = ($rangeEnd + 1) >= $totalSize; + $state['next_offset'] = $rangeEnd + 1; + $state['bytes_received'] = (int) data_get($state, 'bytes_received', 0) + $chunkSize; if (! $isLastChunk) { $this->cache->put($cacheKey, $state, now()->addHours(self::CACHE_TTL_HOURS)); - return [ - 'done' => false, - 'progress' => (int) round(($rangeEnd + 1) / $totalSize * 100), - ]; + return $this->status($state, $totalSize, completed: false); } - $client->completeMultipartUpload([ - 'Bucket' => $bucket, + $this->s3()->completeMultipartUpload([ + 'Bucket' => $this->bucket(), 'Key' => data_get($state, 'key'), 'UploadId' => data_get($state, 'upload_id'), 'MultipartUpload' => [ @@ -117,26 +140,47 @@ public function receiveChunk( $this->cache->forget($cacheKey); + return $this->status($state, $totalSize, completed: true); + } + + /** + * @param array $state + * @return array{done: bool, progress: int, path?: string, size?: int, mime_type?: string} + */ + private function status(array $state, int $totalSize, bool $completed): array + { + $received = (int) data_get($state, 'bytes_received', 0); + $progress = $totalSize > 0 + ? (int) round(min($received, $totalSize) / $totalSize * 100) + : 0; + + if (! $completed) { + return [ + 'done' => false, + 'progress' => $progress, + ]; + } + return [ 'done' => true, 'progress' => 100, 'path' => (string) data_get($state, 'key'), - 'size' => $totalSize, + 'size' => $received, 'mime_type' => (string) data_get($state, 'mime_type'), ]; } /** - * @return array{upload_id: string, key: string, mime_type: string, parts: array} + * @return array{upload_id: string, key: string, mime_type: string, parts: array, next_offset: int, bytes_received: int} */ - private function startUpload(S3Client $client, string $bucket, string $fileName, string $firstChunk): array + private function startUpload(string $fileName, string $firstChunk): array { $extension = strtolower((string) pathinfo($fileName, PATHINFO_EXTENSION)); $key = 'medias/'.Str::uuid().'.'.$extension; $mimeType = $this->detectMimeType($firstChunk, $extension); - $created = $client->createMultipartUpload([ - 'Bucket' => $bucket, + $created = $this->s3()->createMultipartUpload([ + 'Bucket' => $this->bucket(), 'Key' => $key, 'ContentType' => $mimeType, ]); @@ -146,6 +190,8 @@ private function startUpload(S3Client $client, string $bucket, string $fileName, 'key' => $key, 'mime_type' => $mimeType, 'parts' => [], + 'next_offset' => 0, + 'bytes_received' => 0, ]; } diff --git a/tests/Feature/ChunkedCloudUploadTest.php b/tests/Feature/ChunkedCloudUploadTest.php index 54f0cab7..bc0efd60 100644 --- a/tests/Feature/ChunkedCloudUploadTest.php +++ b/tests/Feature/ChunkedCloudUploadTest.php @@ -114,7 +114,7 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0 ->andReturn(new Result([])); $uploader = new ChunkedCloudUploader(Cache::store(), $client, 'test-bucket', 'r2'); - $chunk1 = str_repeat('a', 100); + $chunk1 = str_repeat('a', ChunkedCloudUploader::MIN_PART_BYTES); $chunk2 = str_repeat('b', 50); $total = strlen($chunk1) + strlen($chunk2); @@ -137,6 +137,69 @@ function postChunkedAsset(string $fileName, string $content, int $rangeStart = 0 expect(Cache::get('chunked-cloud-upload:id-1'))->toBeNull(); }); +test('chunked cloud uploader rejects undersized non-final parts', function () { + $uploader = new ChunkedCloudUploader( + Cache::store(), + Mockery::mock(S3Client::class), + 'test-bucket', + 'r2', + ); + + expect(fn () => $uploader->receiveChunk( + 'id-small', + 'video.mp4', + str_repeat('a', 100), + 0, + 99, + ChunkedCloudUploader::MIN_PART_BYTES + 200, + ))->toThrow(InvalidArgumentException::class); +}); + +test('chunked cloud uploader rejects unexpected offsets', function () { + $client = Mockery::mock(S3Client::class); + $client->shouldReceive('createMultipartUpload') + ->once() + ->andReturn(new Result(['UploadId' => 'upload-1'])); + $client->shouldReceive('uploadPart') + ->once() + ->andReturn(new Result(['ETag' => '"etag-a"'])); + + $uploader = new ChunkedCloudUploader(Cache::store(), $client, 'test-bucket', 'r2'); + $chunk1 = str_repeat('a', ChunkedCloudUploader::MIN_PART_BYTES); + $total = ChunkedCloudUploader::MIN_PART_BYTES * 2; + + $uploader->receiveChunk('id-gap', 'video.mp4', $chunk1, 0, strlen($chunk1) - 1, $total); + + expect(fn () => $uploader->receiveChunk( + 'id-gap', + 'video.mp4', + $chunk1, + ChunkedCloudUploader::MIN_PART_BYTES + 10, + ChunkedCloudUploader::MIN_PART_BYTES * 2 + 9, + $total, + ))->toThrow(InvalidArgumentException::class); +}); + +test('chunked cloud uploader is idempotent when a chunk is retried', function () { + $client = Mockery::mock(S3Client::class); + $client->shouldReceive('createMultipartUpload') + ->once() + ->andReturn(new Result(['UploadId' => 'upload-1'])); + $client->shouldReceive('uploadPart') + ->once() + ->andReturn(new Result(['ETag' => '"etag-a"'])); + + $uploader = new ChunkedCloudUploader(Cache::store(), $client, 'test-bucket', 'r2'); + $chunk1 = str_repeat('a', ChunkedCloudUploader::MIN_PART_BYTES); + $total = ChunkedCloudUploader::MIN_PART_BYTES + 50; + + $first = $uploader->receiveChunk('id-retry', 'video.mp4', $chunk1, 0, strlen($chunk1) - 1, $total); + $retry = $uploader->receiveChunk('id-retry', 'video.mp4', $chunk1, 0, strlen($chunk1) - 1, $total); + + expect($first)->toMatchArray(['done' => false]); + expect($retry)->toMatchArray(['done' => false, 'progress' => $first['progress']]); +}); + // ─── HTTP: local / public assemble path ────────────────────────── test('chunked upload stores video on the local disk via assemble path', function () {