From 005d3c9dd6d684a6f17d83b9178b3972b1cbdade Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Thu, 23 Jul 2026 10:55:57 -0300 Subject: [PATCH] fix: stream chunked video finalize to object storage Unicode filename encoding was correct, but large videos still died on the last chunk: the whole file was loaded into memory and uploaded to R2 via Guzzle within PHP-FPM's 30s limit. Stream non-images with writeStream and lift the time limit on finalize. Co-authored-by: Cursor --- app/Http/Controllers/App/AssetController.php | 3 ++ .../App/Asset/StoreChunkedAssetRequest.php | 3 -- app/Models/Traits/HasMedia.php | 46 ++++++++++++++----- resources/js/utils/chunkedUpload.ts | 2 - .../ChunkedUploadFilenameEncodingTest.php | 25 ++++++---- tests/Unit/Traits/HasMediaTest.php | 17 +++++++ 6 files changed, 71 insertions(+), 25 deletions(-) diff --git a/app/Http/Controllers/App/AssetController.php b/app/Http/Controllers/App/AssetController.php index 3997bf40..10eff45b 100644 --- a/app/Http/Controllers/App/AssetController.php +++ b/app/Http/Controllers/App/AssetController.php @@ -98,6 +98,9 @@ public function storeChunked(StoreChunkedAssetRequest $request): JsonResponse ]); } + // Finalize may stream hundreds of MB to object storage (R2/S3). + set_time_limit(0); + $media = $workspace->addMediaFromPath($tempFile, $fileName, 'assets'); @unlink($tempFile); diff --git a/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php b/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php index ca34015b..fbf100c6 100644 --- a/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php +++ b/app/Http/Requests/App/Asset/StoreChunkedAssetRequest.php @@ -28,9 +28,6 @@ protected function prepareForValidation(): void 'range_start' => $parsed[0] ?? null, 'range_end' => $parsed[1] ?? null, 'total_size' => $parsed[2] ?? null, - // Client sends encodeURIComponent(file.name) so unicode filenames - // (en-dash, emoji, …) fit in an ASCII-only HTTP header. Decode - // before lowercasing so `ends_with` sees the real extension. 'file_name' => strtolower(rawurldecode((string) $this->header('X-File-Name', 'upload'))), ]); } diff --git a/app/Models/Traits/HasMedia.php b/app/Models/Traits/HasMedia.php index f2e1752d..e86ab6e0 100644 --- a/app/Models/Traits/HasMedia.php +++ b/app/Models/Traits/HasMedia.php @@ -16,6 +16,7 @@ use Intervention\Image\Drivers\Gd\Driver; use Intervention\Image\Encoders\JpegEncoder; use Intervention\Image\ImageManager; +use RuntimeException; trait HasMedia { @@ -106,6 +107,7 @@ public function addMedia(UploadedFile $file, string $collection = 'default', arr /** * Add media from a file path (used for chunked uploads). + * Non-images are streamed to storage instead of loaded into memory. */ public function addMediaFromPath(string $filePath, string $originalFilename, string $collection = 'default', array $meta = [], ?string $groupId = null): Media { @@ -117,17 +119,37 @@ public function addMediaFromPath(string $filePath, string $originalFilename, str $type = $this->getMediaType($mimeType); $extension = pathinfo($originalFilename, PATHINFO_EXTENSION); - [$normalizedBytes, $normalizedMime, $normalizedExt] = $this->normalizeImageFormat( - $filePath, - $mimeType, - $type, - $extension, - ); + if ($type === Type::Image->value) { + [$bytes, $storedMime, $storedExt] = $this->normalizeImageFormat( + $filePath, + $mimeType, + $type, + $extension, + ); + $size = strlen($bytes); + $mediaMeta = array_merge($this->getMediaMetaFromBytes($bytes, $type, $meta), $meta); + $storagePath = 'medias/'.Str::uuid().'.'.$storedExt; + Storage::put($storagePath, $bytes); + } else { + $storedMime = $mimeType; + $storedExt = $extension; + $size = (int) filesize($filePath); + $mediaMeta = $meta; + $storagePath = 'medias/'.Str::uuid().'.'.$storedExt; + $stream = fopen($filePath, 'rb'); - $filename = Str::uuid().'.'.$normalizedExt; - $storagePath = 'medias/'.$filename; + if ($stream === false) { + throw new RuntimeException("Unable to open media file for reading: {$filePath}"); + } - Storage::put($storagePath, $normalizedBytes); + try { + Storage::writeStream($storagePath, $stream); + } finally { + if (is_resource($stream)) { + fclose($stream); + } + } + } return $this->media()->create([ 'group_id' => $groupId ?? Str::uuid()->toString(), @@ -135,10 +157,10 @@ public function addMediaFromPath(string $filePath, string $originalFilename, str 'type' => $type, 'path' => $storagePath, 'original_filename' => $originalFilename, - 'mime_type' => $normalizedMime, - 'size' => strlen($normalizedBytes), + 'mime_type' => $storedMime, + 'size' => $size, 'order' => 0, - 'meta' => array_merge($this->getMediaMetaFromBytes($normalizedBytes, $type, $meta), $meta), + 'meta' => $mediaMeta, ]); } diff --git a/resources/js/utils/chunkedUpload.ts b/resources/js/utils/chunkedUpload.ts index 2a3c536e..8e6e6a6f 100644 --- a/resources/js/utils/chunkedUpload.ts +++ b/resources/js/utils/chunkedUpload.ts @@ -49,8 +49,6 @@ export const uploadChunked = async (options: ChunkedUploadOptions): Promise = { 'Content-Type': 'application/octet-stream', 'Content-Range': `bytes ${start}-${end - 1}/${totalSize}`, - // Headers must be ByteString (≤255). encodeURIComponent keeps - // unicode filenames (en-dash, emoji, …) ASCII-safe for fetch(). 'X-File-Name': encodeURIComponent(file.name), 'X-CSRF-TOKEN': csrfToken, 'X-Requested-With': 'XMLHttpRequest', diff --git a/tests/Feature/ChunkedUploadFilenameEncodingTest.php b/tests/Feature/ChunkedUploadFilenameEncodingTest.php index cc304af7..5d0b6886 100644 --- a/tests/Feature/ChunkedUploadFilenameEncodingTest.php +++ b/tests/Feature/ChunkedUploadFilenameEncodingTest.php @@ -32,10 +32,6 @@ ]); }); -/** - * Mimic the browser client: encodeURIComponent → PHP rawurlencode for the - * X-File-Name header value. - */ function postEncodedChunkedUpload(string $fileName, string $content): TestResponse { $size = strlen($content); @@ -55,8 +51,6 @@ function postEncodedChunkedUpload(string $fileName, string $content): TestRespon } test('chunked upload accepts filename with en-dash when percent-encoded', function () { - // Customer bug: en-dash (U+2013) is outside ISO-8859-1, so fetch() rejects - // the raw X-File-Name header. The client percent-encodes; we decode here. $fileName = 'Corte 6 – Quantidade ou qualidade_ Os dois..png'; $content = file_get_contents(__DIR__.'/../fixtures/1x1.png'); @@ -75,7 +69,6 @@ function postEncodedChunkedUpload(string $fileName, string $content): TestRespon $response = postEncodedChunkedUpload($fileName, $content); $response->assertSuccessful(); - $response->assertJson(['done' => true]); expect($this->workspace->getMedia('assets')->first()->original_filename) ->toBe(strtolower($fileName)); }); @@ -92,7 +85,6 @@ function postEncodedChunkedUpload(string $fileName, string $content): TestRespon }); test('chunked upload still accepts plain ascii filename without encoding', function () { - // Backwards compatible: rawurldecode is a no-op on plain ASCII names. $content = file_get_contents(__DIR__.'/../fixtures/1x1.png'); $size = strlen($content); @@ -119,3 +111,20 @@ function postEncodedChunkedUpload(string $fileName, string $content): TestRespon $response->assertUnprocessable(); }); + +test('chunked upload streams a video file to storage on finalize', function () { + // Minimal ISO BMFF ("ftyp") so mime_content_type reports video/mp4. + $content = "\0\0\0\x18ftypmp42\0\0\0\0mp42isom".str_repeat("\0", 64); + $fileName = 'Quantidade ou Qualidade_ Os dois..mp4'; + + $response = postEncodedChunkedUpload($fileName, $content); + + $response->assertSuccessful(); + $response->assertJson(['done' => true, 'type' => 'video']); + + $media = $this->workspace->getMedia('assets')->first(); + expect($media->original_filename)->toBe(strtolower($fileName)); + expect($media->type->value)->toBe('video'); + expect($media->size)->toBe(strlen($content)); + Storage::assertExists($media->path); +}); diff --git a/tests/Unit/Traits/HasMediaTest.php b/tests/Unit/Traits/HasMediaTest.php index 7380fc51..87a658ca 100644 --- a/tests/Unit/Traits/HasMediaTest.php +++ b/tests/Unit/Traits/HasMediaTest.php @@ -118,6 +118,23 @@ unlink($tempFile); }); +test('model can add video media from file path via stream', function () { + $workspace = Workspace::factory()->create(); + + $tempFile = tempnam(sys_get_temp_dir(), 'vid'); + $bytes = "\0\0\0\x18ftypmp42\0\0\0\0mp42isom".str_repeat("\0", 64); + file_put_contents($tempFile, $bytes); + + $media = $workspace->addMediaFromPath($tempFile, 'clip.mp4', 'assets'); + + expect($media->type->value)->toBe('video'); + expect($media->size)->toBe(strlen($bytes)); + expect($media->mime_type)->toBe('video/mp4'); + Storage::assertExists($media->path); + + unlink($tempFile); +}); + test('model can clear media collection', function () { $workspace = Workspace::factory()->create(); $file1 = UploadedFile::fake()->image('logo1.jpg', 100, 100);